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

Having loom as dev-dependency incompatible with loom integration tests #134

Closed
faern opened this issue Apr 26, 2020 · 2 comments · Fixed by #137
Closed

Having loom as dev-dependency incompatible with loom integration tests #134

faern opened this issue Apr 26, 2020 · 2 comments · Fixed by #137

Comments

@faern
Copy link
Contributor

faern commented Apr 26, 2020

The main readme recommends adding loom as a dev-dependency. Then it goes on to mention having loom tests as integration tests:

For example, if the library includes a test file: tests/loom_my_struct.rs ...

This does not work. Because when building integration tests the main library is not built in #[cfg(test)] mode but rather in the same way cargo build builds it. So the dev-dependencies are not available. As such, the crate can't use loom::thread::spawn etc internally.

Would you recommend only doing loom tests in unit tests? This is what I have seen some of the libraries using loom doing. Or having loom as a normal, but optional, dependency under [dependencies]? Then anyone can activate them when they use the crate. May be good or bad. The bad part is that the crate exposes features normal users are not meant to use. The good part is If the crate itself implements synchronization primitives, then users of that crate can activate the loom feature if they want to loom test their stuff depending on the synchronization primitives in this library.

carllerche added a commit that referenced this issue Apr 28, 2020
@carllerche
Copy link
Member

Does this change work? #137

carllerche added a commit that referenced this issue Apr 28, 2020
@faern
Copy link
Contributor Author

faern commented Apr 29, 2020

That's better, thanks!

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 a pull request may close this issue.

2 participants