-
Notifications
You must be signed in to change notification settings - Fork 8
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
sync feature causes tests to fail #10
Comments
This is a problem with the unit tests + how cargo does additive features. The sync and and default feature set cannot be enabled at the same time. By default, it uses a thread-local variable for the clock. With the sync feature enabled, it uses a global thread-aware clock. So, with sync enabled + the default feature (which should be subtractive but cargo can't express that) during tests it uses the thread-local source for the thread-global clock. I should be able to fix this by duplicated the unit tests with the right feature flags to ensure there's a _sync variant for each test. This crate isn't intended to be ran with all of the features. e.g. it should be a compile-time error but there isn't a way to express that with how the features are set up. I can add a 2nd feature, which is the default feature ( |
https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#added-46 I see they added the |
It'll produce an error such as:
By default, the (new) thread_local feature will be enabled. To run the unit tests just for the While:
To be quite honest, I'm surprised they added May I ask why you need to pass
Rather than all features. I can perhaps come up with a weird |
Currently |
I see, I'll just duplicate the tests, I may change how the features work. It'll be in a few hours, but I think I may remove the thread local config, it doesn't really make sense now that one can't run the tests in the current thread. |
I was having a lot of problems with git, wine during dinner didn't help. But eventually I figured out why the histories were all disjoint. I published https://crates.io/crates/mock_instant/0.4.0 which removes the hard-to-use thread-local clock. Now the tests should pass, and one less dependency. And this mock clock should be usable across unit-test thread invocations. Thanks again, there was a bug in the unit tests added when the mockable |
Thanks for the quick resolution! |
cargo test
works fine in the repo as is, but running it for all features causes the time checks to choke.Here is the full stacktrace;
The text was updated successfully, but these errors were encountered: