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

Add support for global and group slot numbers #2145

Merged
merged 3 commits into from
Feb 12, 2025
Merged

Add support for global and group slot numbers #2145

merged 3 commits into from
Feb 12, 2025

Conversation

sunshowers
Copy link
Member

Per #2142 -- add support for global and group slots.

This should address a class of problems around things like port number allocations. For example, if you want to reserve blocks of 5 ports for tests, you could pick an arbitrary starting port in the ephemeral port range (e.g. 46210) and then do starting_port + 5 * NEXTEST_TEST_GROUP_SLOT.

These slots are numbers increasing from 0, are unique for the lifetime of the test within the nextest run (or within the group), and are compact (numbers are freed as tests finish, and the smallest possible number is always assigned to new tests).

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 9 lines in your changes missing coverage. Please review.

Project coverage is 80.11%. Comparing base (ffd6a8c) to head (938c5f1).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
nextest-runner/src/runner/imp.rs 89.36% 5 Missing ⚠️
nextest-runner/src/runner/executor.rs 40.00% 3 Missing ⚠️
nextest-runner/src/config/test_group.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2145      +/-   ##
==========================================
- Coverage   80.12%   80.11%   -0.02%     
==========================================
  Files         103      103              
  Lines       24343    24370      +27     
==========================================
+ Hits        19505    19524      +19     
- Misses       4838     4846       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

trunk-io bot commented Feb 12, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@sondrelg
Copy link

sondrelg commented Feb 12, 2025

Thanks again for adding this @sunshowers!

I tried cloning and checking out 9977b2a; building a release version, then running the following test:

#[test]
fn l() {
    println!("{}", std::env::var("NEXTEST_TEST_GROUP_SLOT").unwrap());
}

with ./nextest/target/release/cargo-nextest nextest run tests::l

Unexpectedly, the test fails, since the env var is undefined:

──── STDERR:             print tests::l
thread 'tests::l' panicked at src/lib.rs:142:60:
called `Result::unwrap()` on an `Err` value: NotPresent
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

  Cancelling due to test failure
────────────
     Summary [   0.011s] 1 test run: 0 passed, 1 failed, 35 skipped
        FAIL [   0.009s] print tests::l
error: test run failed

Am I doing something wrong?

--

EDIT: I typoed NEXTEST_TEST_GLOBAL_SLOT 🙈 Running three concurrent tests, I see 0, 1, and 2 for NEXTEST_TEST_GROUP 🥳

@sunshowers
Copy link
Member Author

Thanks for testing it out! This suggests that merely removing the GROUP variables from the environment isn't enough, we should probably set them to values which indicate that the test is not in a group.

…SLOT

Set these to environment variables that are *unique* for the lifetime of the
test, and *compact* (smallest possible number can be assigned).
Expose the group name that the test is running under, if any.
@sunshowers sunshowers merged commit 75f6e57 into main Feb 12, 2025
20 checks passed
@sunshowers sunshowers deleted the slots branch February 12, 2025 22:31
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.

2 participants