-
Notifications
You must be signed in to change notification settings - Fork 318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean up tests that need to shut down transaction status service. #4654
Conversation
bef7502
to
fd533a0
Compare
@steviez I tried to make the new function dcou, but ran into an issue with 'features' not allowed in 'svm/examples' toml file, which would be needed in addition to adding solana-rpc as a dev dependency. So I left it out for now. |
That is surprising to me. I don't see It looks like you force-pushed and blew away your old commits so I can't see what you had. Did you see that failure reported in CI or during local build ? |
feature not allowed was on a local build. But it appears that was more of a skill issue on my part...I think we are good to go with the latest patch for making it dcou (asuming it clears CI). Thx for quick review and response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few nits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Unify the wind down of transaction status service across different unit tests to ensure more consistency, and make sure all tx statuses are processed before the verification step of each test.
This change was forked from agave PR #4032 (specifically these comments here and here for context).