Skip to content
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

Fix async tests by using tokio runtime #87

Closed
wants to merge 2 commits into from

Conversation

liamsi
Copy link
Member

@liamsi liamsi commented Dec 11, 2019

Uses tokio runtime builder to create a block_on method (depends on #85).

Tested locally by re-enabling some of the ignored tests: most pass, some do not. Looks like the JSON format changed in the meantime.

@liamsi liamsi changed the base branch from master to rpc/async December 11, 2019 09:14
@liamsi liamsi requested a review from tarcieri December 11, 2019 09:18
 - delete futures crate from Cargo.toml
 - add tokio as a dev dependency
Copy link
Contributor

@xla xla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dope. The only thing that we need to be aware of is, if there is another runtime in the system which doesn't share the same pool, it can lead to halts and locks.

👍 🍡 🐼

@liamsi
Copy link
Member Author

liamsi commented Dec 11, 2019

Thanks for reviewing @xla!

The only thing that we need to be aware of is, if there is another runtime in the system which doesn't share the same pool, it can lead to halts and locks.

Is this sth. we should be concerned about in the context of this PR? Is that easy to get wrong in tests (create multiple Runtimes with distinct thread-pools)? I do not understand enough about the internals of the runtime to understand when we would need to worry about this.

@liamsi
Copy link
Member Author

liamsi commented Dec 11, 2019

This got manually merged into #85 (comment). Closing.

@liamsi liamsi closed this Dec 11, 2019
@liamsi liamsi deleted the fix_tests/rpc/async branch December 11, 2019 13:54
@liamsi liamsi restored the fix_tests/rpc/async branch December 11, 2019 13:55
@liamsi liamsi deleted the fix_tests/rpc/async branch December 11, 2019 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants