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

sync feature causes tests to fail #10

Closed
WeepingClown13 opened this issue Apr 4, 2024 · 7 comments
Closed

sync feature causes tests to fail #10

WeepingClown13 opened this issue Apr 4, 2024 · 7 comments

Comments

@WeepingClown13
Copy link

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;

$ RUST_BACKTRACE=full cargo test --all-features 
    Finished test [unoptimized + debuginfo] target(s) in 0.00s
     Running unittests src/lib.rs (target/debug/deps/mock_instant-44d1854359e58544)

running 9 tests
test tests::advance ... ok
test tests::advance_system_time ... ok
test tests::instant ... ok
test tests::set_system_time ... ok
test tests::set_time ... ok
test tests::system_time ... ok
test tests::system_time_from_std_roundtrip ... ok
test tests::methods ... FAILED
test tests::system_time_methods ... FAILED

failures:

---- tests::methods stdout ----
thread 'tests::methods' panicked at 'assertion failed: `(left == right)`
  left: `Instant(701ms)`,
 right: `Instant(1ms)`', src/lib.rs:511:9
stack backtrace:
   0:     0x55900b5a0c9c - std::backtrace_rs::backtrace::libunwind::trace::he2ba3a4891b10ef3
                               at /usr/src/rustc-1.63.0/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55900b5a0c9c - std::backtrace_rs::backtrace::trace_unsynchronized::ha0fda2e57da4b2a3
                               at /usr/src/rustc-1.63.0/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55900b5a0c9c - std::sys_common::backtrace::_print_fmt::hbfe6e1f0cd4bb862
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x55900b5a0c9c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h82b6828459151f7c
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x55900b5b57ae - core::fmt::write::hafcd92e27b23e937
                               at /usr/src/rustc-1.63.0/library/core/src/fmt/mod.rs:1197:17
   5:     0x55900b582051 - std::io::Write::write_fmt::hb4885aa3caa0231c
                               at /usr/src/rustc-1.63.0/library/std/src/io/mod.rs:1672:15
   6:     0x55900b584b8e - std::sys_common::backtrace::_print::h9a164f1073e1bcc5
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x55900b584b8e - std::sys_common::backtrace::print::hb860acc8c631da42
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x55900b584b8e - std::panicking::default_hook::{{closure}}::h2c2be97328f88741
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:295:22
   9:     0x55900b584836 - std::panicking::default_hook::h44f9af4dc0ebff0f
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:311:9
  10:     0x55900b5850a1 - std::panicking::rust_panic_with_hook::h57071e38e2bc223f
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:698:17
  11:     0x55900b5a19c7 - std::panicking::begin_panic_handler::{{closure}}::h7ff3a0ebbf1ba422
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:588:13
  12:     0x55900b5a0db4 - std::sys_common::backtrace::__rust_end_short_backtrace::hc8542ca3b5dac53a
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:138:18
  13:     0x55900b584d52 - rust_begin_unwind
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:584:5
  14:     0x55900b52c5f3 - core::panicking::panic_fmt::h11223f0b8c31003a
                               at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:142:14
  15:     0x55900b5b7638 - core::panicking::assert_failed_inner::hf6f06ba4e137dab8
  16:     0x55900b52f99a - core::panicking::assert_failed::h18551bcfbf99e1d2
                               at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:181:5
  17:     0x55900b531cce - mock_instant::tests::methods::hcae17ca62040a408
                               at /home/weepingclown/debian/rust/mock_instant/src/lib.rs:511:9
  18:     0x55900b52eaca - mock_instant::tests::methods::{{closure}}::hd4b04fd3fbb0a62c
                               at /home/weepingclown/debian/rust/mock_instant/src/lib.rs:487:5
  19:     0x55900b52ee8e - core::ops::function::FnOnce::call_once::h5b8b0bf6a7bdfe85
                               at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  20:     0x55900b53b3e3 - core::ops::function::FnOnce::call_once::h5bc720900f914b9d
                               at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  21:     0x55900b53b3e3 - test::__rust_begin_short_backtrace::hf3b206c6e88326de
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:572:5
  22:     0x55900b53b4fd - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::he4d1804912c93e58
                               at /usr/src/rustc-1.63.0/library/alloc/src/boxed.rs:1951:9
  23:     0x55900b53b4fd - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::hd51627ecae6da161
                               at /usr/src/rustc-1.63.0/library/core/src/panic/unwind_safe.rs:271:9
  24:     0x55900b53b4fd - std::panicking::try::do_call::h50cc18c73c99298e
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:492:40
  25:     0x55900b53b4fd - std::panicking::try::hd680651bc1ffe9d2
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:456:19
  26:     0x55900b53b4fd - std::panic::catch_unwind::hac2e6452375597b8
                               at /usr/src/rustc-1.63.0/library/std/src/panic.rs:137:14
  27:     0x55900b53b4fd - test::run_test_in_process::hf986b64d597e186b
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:595:18
  28:     0x55900b560604 - test::run_test::run_test_inner::{{closure}}::h8d94037939575cbe
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:489:39
  29:     0x55900b560604 - test::run_test::run_test_inner::{{closure}}::ha13af21b3709e767
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:516:37
  30:     0x55900b560604 - std::sys_common::backtrace::__rust_begin_short_backtrace::hac1918f21a054110
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:122:18
  31:     0x55900b561656 - std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}::h19f4ccd903b8269a
                               at /usr/src/rustc-1.63.0/library/std/src/thread/mod.rs:505:17
  32:     0x55900b561656 - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h7abf11856c7fd547
                               at /usr/src/rustc-1.63.0/library/core/src/panic/unwind_safe.rs:271:9
  33:     0x55900b561656 - std::panicking::try::do_call::h180ce190389c4cd5
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:492:40
  34:     0x55900b561656 - std::panicking::try::hdee150d395570877
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:456:19
  35:     0x55900b561656 - std::panic::catch_unwind::h21da9f980d906437
                               at /usr/src/rustc-1.63.0/library/std/src/panic.rs:137:14
  36:     0x55900b561656 - std::thread::Builder::spawn_unchecked_::{{closure}}::h46be2c515e98c836
                               at /usr/src/rustc-1.63.0/library/std/src/thread/mod.rs:504:30
  37:     0x55900b561656 - core::ops::function::FnOnce::call_once{{vtable.shim}}::hd2113b15344880f2
                               at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  38:     0x55900b58d563 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hf85a8d964ad6b861
                               at /usr/src/rustc-1.63.0/library/alloc/src/boxed.rs:1951:9
  39:     0x55900b58d563 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::ha33be44d0848e316
                               at /usr/src/rustc-1.63.0/library/alloc/src/boxed.rs:1951:9
  40:     0x55900b58d563 - std::sys::unix::thread::Thread::new::thread_start::hee65e032a8d102e4
                               at /usr/src/rustc-1.63.0/library/std/src/sys/unix/thread.rs:108:17
  41:     0x7f624ae9a134 - <unknown>
  42:     0x7f624af1a7dc - <unknown>
  43:                0x0 - <unknown>

---- tests::system_time_methods stdout ----
thread 'tests::system_time_methods' panicked at 'assertion failed: `(left == right)`
  left: `SystemTime(401ms)`,
 right: `SystemTime(1ms)`', src/lib.rs:410:9
stack backtrace:
   0:     0x55900b5a0c9c - std::backtrace_rs::backtrace::libunwind::trace::he2ba3a4891b10ef3
                               at /usr/src/rustc-1.63.0/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55900b5a0c9c - std::backtrace_rs::backtrace::trace_unsynchronized::ha0fda2e57da4b2a3
                               at /usr/src/rustc-1.63.0/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55900b5a0c9c - std::sys_common::backtrace::_print_fmt::hbfe6e1f0cd4bb862
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x55900b5a0c9c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h82b6828459151f7c
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x55900b5b57ae - core::fmt::write::hafcd92e27b23e937
                               at /usr/src/rustc-1.63.0/library/core/src/fmt/mod.rs:1197:17
   5:     0x55900b582051 - std::io::Write::write_fmt::hb4885aa3caa0231c
                               at /usr/src/rustc-1.63.0/library/std/src/io/mod.rs:1672:15
   6:     0x55900b584b8e - std::sys_common::backtrace::_print::h9a164f1073e1bcc5
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x55900b584b8e - std::sys_common::backtrace::print::hb860acc8c631da42
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x55900b584b8e - std::panicking::default_hook::{{closure}}::h2c2be97328f88741
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:295:22
   9:     0x55900b584836 - std::panicking::default_hook::h44f9af4dc0ebff0f
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:311:9
  10:     0x55900b5850a1 - std::panicking::rust_panic_with_hook::h57071e38e2bc223f
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:698:17
  11:     0x55900b5a19c7 - std::panicking::begin_panic_handler::{{closure}}::h7ff3a0ebbf1ba422
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:588:13
  12:     0x55900b5a0db4 - std::sys_common::backtrace::__rust_end_short_backtrace::hc8542ca3b5dac53a
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:138:18
  13:     0x55900b584d52 - rust_begin_unwind
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:584:5
  14:     0x55900b52c5f3 - core::panicking::panic_fmt::h11223f0b8c31003a
                               at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:142:14
  15:     0x55900b5b7638 - core::panicking::assert_failed_inner::hf6f06ba4e137dab8
  16:     0x55900b52fb1a - core::panicking::assert_failed::h9dd40dd001aa02cb
                               at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:181:5
  17:     0x55900b530bd7 - mock_instant::tests::system_time_methods::h62dbdc88f10088c6
                               at /home/weepingclown/debian/rust/mock_instant/src/lib.rs:410:9
  18:     0x55900b52ea2a - mock_instant::tests::system_time_methods::{{closure}}::h800b7fadbc666f1b
                               at /home/weepingclown/debian/rust/mock_instant/src/lib.rs:404:5
  19:     0x55900b52ef1e - core::ops::function::FnOnce::call_once::h7d48b10de78289af
                               at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  20:     0x55900b53b3e3 - core::ops::function::FnOnce::call_once::h5bc720900f914b9d
                               at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  21:     0x55900b53b3e3 - test::__rust_begin_short_backtrace::hf3b206c6e88326de
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:572:5
  22:     0x55900b53b4fd - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::he4d1804912c93e58
                               at /usr/src/rustc-1.63.0/library/alloc/src/boxed.rs:1951:9
  23:     0x55900b53b4fd - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::hd51627ecae6da161
                               at /usr/src/rustc-1.63.0/library/core/src/panic/unwind_safe.rs:271:9
  24:     0x55900b53b4fd - std::panicking::try::do_call::h50cc18c73c99298e
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:492:40
  25:     0x55900b53b4fd - std::panicking::try::hd680651bc1ffe9d2
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:456:19
  26:     0x55900b53b4fd - std::panic::catch_unwind::hac2e6452375597b8
                               at /usr/src/rustc-1.63.0/library/std/src/panic.rs:137:14
  27:     0x55900b53b4fd - test::run_test_in_process::hf986b64d597e186b
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:595:18
  28:     0x55900b560604 - test::run_test::run_test_inner::{{closure}}::h8d94037939575cbe
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:489:39
  29:     0x55900b560604 - test::run_test::run_test_inner::{{closure}}::ha13af21b3709e767
                               at /usr/src/rustc-1.63.0/library/test/src/lib.rs:516:37
  30:     0x55900b560604 - std::sys_common::backtrace::__rust_begin_short_backtrace::hac1918f21a054110
                               at /usr/src/rustc-1.63.0/library/std/src/sys_common/backtrace.rs:122:18
  31:     0x55900b561656 - std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}::h19f4ccd903b8269a
                               at /usr/src/rustc-1.63.0/library/std/src/thread/mod.rs:505:17
  32:     0x55900b561656 - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h7abf11856c7fd547
                               at /usr/src/rustc-1.63.0/library/core/src/panic/unwind_safe.rs:271:9
  33:     0x55900b561656 - std::panicking::try::do_call::h180ce190389c4cd5
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:492:40
  34:     0x55900b561656 - std::panicking::try::hdee150d395570877
                               at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:456:19
  35:     0x55900b561656 - std::panic::catch_unwind::h21da9f980d906437
                               at /usr/src/rustc-1.63.0/library/std/src/panic.rs:137:14
  36:     0x55900b561656 - std::thread::Builder::spawn_unchecked_::{{closure}}::h46be2c515e98c836
                               at /usr/src/rustc-1.63.0/library/std/src/thread/mod.rs:504:30
  37:     0x55900b561656 - core::ops::function::FnOnce::call_once{{vtable.shim}}::hd2113b15344880f2
                               at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  38:     0x55900b58d563 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hf85a8d964ad6b861
                               at /usr/src/rustc-1.63.0/library/alloc/src/boxed.rs:1951:9
  39:     0x55900b58d563 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::ha33be44d0848e316
                               at /usr/src/rustc-1.63.0/library/alloc/src/boxed.rs:1951:9
  40:     0x55900b58d563 - std::sys::unix::thread::Thread::new::thread_start::hee65e032a8d102e4
                               at /usr/src/rustc-1.63.0/library/std/src/sys/unix/thread.rs:108:17
  41:     0x7f624ae9a134 - <unknown>
  42:     0x7f624af1a7dc - <unknown>
  43:                0x0 - <unknown>


failures:
    tests::methods
    tests::system_time_methods

test result: FAILED. 7 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass `--lib`
@museun
Copy link
Owner

museun commented Apr 4, 2024

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 (no_sync or similar) that way if one were to use --all-features or the equivalent it'll produce an compile-time error.

@museun
Copy link
Owner

museun commented Apr 4, 2024

https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#added-46 I see they added the --all-features flag since I wrote this crate, I'll take that into account and fix the feature matrix in a backwards compatible fashion

@museun
Copy link
Owner

museun commented Apr 5, 2024

It'll produce an error such as:

dev [=$+?] mock_instant ❯ cargo check --no-default-features
    Checking mock_instant v0.2.1 (F:\rs\mock_instant)
error: feature `sync` or `thread_local` (default) must be enabled
  --> src\lib.rs:53:5
   |
53 |     compile_error!("feature `sync` or `thread_local` (default) must be enabled");
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `mock_instant` (lib) due to 1 previous error
dev [=$+?] mock_instant ✖ cargo check -F sync -F thread_local
    Checking mock_instant v0.2.1 (F:\rs\mock_instant)
error: feature `sync` or `thread_local` (default) must be enabled
  --> src\lib.rs:53:5
   |
53 |     compile_error!("feature `sync` or `thread_local` (default) must be enabled");    
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^     

error: could not compile `mock_instant` (lib) due to 1 previous error

By default, the (new) thread_local feature will be enabled.

To run the unit tests just for the sync feature then cargo test --no-default-features -F sync must be used (cargo test will run the unit tests for the thread_local clock)

While:

dev [=$!+?] mock_instant ✖ cargo test --all-features
   Compiling mock_instant v0.2.1 (F:\rs\mock_instant)
error: feature `sync` or `thread_local` (default) must be enabled                         
  --> src\lib.rs:53:5
   |
53 |     compile_error!("feature `sync` or `thread_local` (default) must be enabled");    
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^     

error: could not compile `mock_instant` (lib) due to 1 previous error                     
warning: build failed, waiting for other jobs to finish...
error: could not compile `mock_instant` (lib test) due to 1 previous error
dev [=$!+?] mock_instant ✖

--all-features will produce an compile-time error.

To be quite honest, I'm surprised they added --all-features (its a footgun. there used to be a cargo plugin that did this but ran into a lot of ecosystem problems)

May I ask why you need to pass --all-features to the test harness for this crate? If its for a vendored CI run, then changes (after applying this 'fix') would have to be made to your CI config to run

  • cargo test --no-default-features -F sync
  • cargo test --no-default-features -F thread-local

Rather than all features. I can perhaps come up with a weird #[cfg()] combination so cargo test will test them all now that I don't particularly need OnceCell as a dep any more.

@WeepingClown13
Copy link
Author

Currently cargo test --no-default-features -F sync fails as well, I guess I will have to wait for your change. As I understand it, there is no way to run the tests for the sync feature currently. FYI, I have no inclination towards --all-features and is nor much of a knowledgeable person in this matter, it is just that I came across the situation where the test is failing when I was packaging the crate for debian and that particular flag simply allowed me to reproduce the issue very easily.

@museun
Copy link
Owner

museun commented Apr 7, 2024

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.

rust-lang/rust#104053

@museun museun closed this as completed in 4139519 Apr 7, 2024
@museun
Copy link
Owner

museun commented Apr 7, 2024

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 SystemTime was added. I added a CI to catch this stuff.

@WeepingClown13
Copy link
Author

Thanks for the quick resolution!

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

No branches or pull requests

2 participants