Skip to content

Consider using cargo-nextest #1629

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

Open
ogenev opened this issue Jan 10, 2025 · 3 comments
Open

Consider using cargo-nextest #1629

ogenev opened this issue Jan 10, 2025 · 3 comments

Comments

@ogenev
Copy link
Member

ogenev commented Jan 10, 2025

cargo-nexttest looks very promising -https://nexte.st/.

The only downside is that it cannot run doc tests, but we are not using them anyway.

@CPerezz
Copy link

CPerezz commented Mar 16, 2025

Hey!

I've been giving this a try today.
So far the change is easy. There's just one catch.

cargo-nextest will by default run on all available threads.
This is fine for most of the codebase except for ethportal-peertest::self_peertest a d ethportal-peertest::rpc_server groups of tests.

In these, we actually need to launch a trin client. And the problem is that once one test takes the ownership of the file-descriptor which holds the IO address for the listener of trin, the rest of tests panic due to the error of the address to listen to being already taken:

trin/testing/ethportal-peertest/src/lib.rs:45:56:
called `Result::unwrap()` on an `Err` value: "Failed to start discv5 server: Io(Os { code: 48, kind: AddrInUse, message: \"Address already in use\" })"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I tried to go the setup/teardown route. Ideally you just need to setup
https://github.com/CPerezz/trin/blob/9d8ba360359d9dab9e4226fd16161463ec730b67/testing/ethportal-peertest/tests/self_peertest.rs#L373`
and the rest is about sharing the instance cross-tests. But static variables or crates like ctor or lazy_static don't actually support async/await.

Retunring the Future (which is valid behaviour within static for example) to then call await within the test would be ok. The problem is that the Future won't actually work as the syncronisation doesn't happen.

What I can definitely say is that the speedup in test execution is notable. Is that the only thing you were interested?

cc: @ogenev

@ogenev
Copy link
Member Author

ogenev commented Mar 17, 2025

@CPerezz thanks for the info, should we run ethportal-peertest with the default test runner and the rest of the crates with cargo-nextest?

@CPerezz
Copy link

CPerezz commented Mar 18, 2025

@ogenev So I made a solution with a Mutex to do common setup for all tests (that share Subnetwork config ofc).
So far, something is not right as the Mutex seems to not be doing anything.

I understood why. cargo-nextest is getting each test binary and launching it in parallel with tokio. That means, that each test has no context whatsoever with the others.
Hence, any Mutex or static I create will simply be worthless.

Give me a couple days to actually see if I can unify the setup/teardown. And just rely on cargo-nextest. Otherwise yes, the simplest solution will be to use both test runners.
cargo-nextest has experimental support for scripts. So will explore it prior to calling it a day.

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