Skip to content

Conversation

@pwithams
Copy link

PR for #1744.

Total test runtime appears very slightly slower (by ~100ms for me locally), presumably a constant due to the overhead of creating separate binaries for each test file. However, I think the improved organization is probably worth it as it makes it a little clearer how the tests are distributed and to know where to put new tests.

As each test file is created as a separate binary now clippy warns when parts of testenv are not used by every test file, so I added #![allow(dead_code)] at the top of testenv/mod.rs to suppress that. Not sure if there's a better way to handle that.

I'm open to any suggestions for different file layouts too. For example, if there any gaps in current test types a file could be created to highlight that with the goal of filling it out.

@pwithams pwithams mentioned this pull request Jul 4, 2025
@tmccombs
Copy link
Collaborator

tmccombs commented Jul 6, 2025

We could potentially make a single create with mutiple files for the integration tests, like https://momori.dev/posts/organize-rust-integration-tests-without-dead-code-warning/#one-integration-test-crate-with-modules

I'm not sure if the is the right thing to do here or not.

@pwithams
Copy link
Author

pwithams commented Jul 9, 2025

Yeah I like that idea. I see that cargo uses a similar approach:

rust-lang/cargo#5022
https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html

Some timings after reorganizing - seems runtime is looking good again:

Original HEAD: cargo test 3.70s user 1.46s system 539% cpu 0.956 total
Splitting tests into separate files/binaries: cargo test 3.51s user 1.40s system 448% cpu 1.096 total
Using a single crate for tests: cargo test 3.74s user 1.39s system 584% cpu 0.879 total

I renamed the original tests/tests.rs file to tests/integration/general.rs to try make it clear each file is a related grouping of tests.

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