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

Remove nervosnetwork/faketime, replace cargo test by nextest-rs/nextest #3777

Merged
merged 3 commits into from
Jan 16, 2023

Conversation

eval-exec
Copy link
Collaborator

@eval-exec eval-exec commented Dec 28, 2022

What problem does this PR solve?

Issue Number: close #3729

Problem Summary:

This PR want to remove nervosnetwork/faketime, and simply use static atomics and nextest to organize ckb's unit tests.

What is changed and how it works?

The nervosnetwork/faketime is complicated, and when we specified --test-threads=1 flag to cargo test, these thread_local (
https://github.com/nervosnetwork/faketime/blob/1ead0aba139f0893d04aa95c35e0179c3b4482c7/src/faketime.rs#L109-L115 ) behave like global variables, then some tests will fail.

The cargo test run all unit test in a single process, and many unit tests may run on a same thread. And there exists some faketime related test want to change the whole process's faketime by set current process's environment like this:

::std::env::set_var("FAKETIME", faketime_file.as_os_str());

this may affect other non-faketime related unit tests.

Fortunately, there is nextest-rs/nextest which will run every unit function on single process, providing process-level isolation granularity.

What's Changed:

Related changes

  • remove nervosnetwork/faketime
  • add util/systemtime to provide real systemtime, and provide faketime when enable_faketime feature is enabled
  • use nextest-rs/nextest to run all unit tests

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code ci-runs-only: [ quick_checks,linters ]

Side effects

  • None

Release note

Title Only: Include only the PR title in the release note.

@eval-exec eval-exec changed the title [WIP] remove nervosnetwork/faketime, use serial with static atomics to provide [WIP] remove nervosnetwork/faketime, replace cargo test by nextest-rs/nextest Dec 29, 2022
@eval-exec eval-exec force-pushed the fix/mock-time branch 14 times, most recently from 464e587 to 4b08302 Compare December 29, 2022 08:56
@eval-exec
Copy link
Collaborator Author

eval-exec commented Dec 29, 2022

Try to make github ci action use cargo-nextest.
But get an error on Windows platform:
https://github.com/nervosnetwork/ckb/actions/runs/3799433532/jobs/6461954529

error: failed to compile `cargo-nextest v0.9.47`, intermediate artifacts can be found at `C:\Users\ADMINI~1\AppData\Local\Temp\cargo-installaoztQa`

Caused by:
  package `cargo-nextest v0.9.47` cannot be built because it requires rustc 1.62 or newer, while the currently active rustc version is 1.61.0
Error: Process completed with exit code 1.

Trying to install an older version of cargo-nextest on ci_*_windows

@eval-exec eval-exec marked this pull request as ready for review December 29, 2022 09:14
@eval-exec eval-exec requested a review from a team as a code owner December 29, 2022 09:14
@eval-exec eval-exec force-pushed the fix/mock-time branch 9 times, most recently from 0088c7c to 61abbd9 Compare December 29, 2022 14:01
@eval-exec eval-exec force-pushed the fix/mock-time branch 6 times, most recently from 6a79801 to 27cf77b Compare December 29, 2022 15:48
@eval-exec eval-exec changed the title [WIP] remove nervosnetwork/faketime, replace cargo test by nextest-rs/nextest Remove nervosnetwork/faketime, replace cargo test by nextest-rs/nextest Dec 30, 2022
@eval-exec eval-exec force-pushed the fix/mock-time branch 2 times, most recently from 4240ccc to 4e9fa6f Compare January 10, 2023 01:44
@zhangsoledad
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 16, 2023

@bors bors bot merged commit 93b318a into nervosnetwork:develop Jan 16, 2023
@eval-exec eval-exec deleted the fix/mock-time branch April 7, 2023 02:00
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.

Fix and simplify timestamp-related tests
2 participants