Skip to content

Commit

Permalink
[nextest-runner] use a setup script to seed build for integration tes…
Browse files Browse the repository at this point in the history
…ts (#1963)

Currently, all our integration tests build a separate copy of the nextest
fixtures. This is a lot of unnecessary extra build work.

We now have a great solution, though -- use setup scripts! This kind of build
caching is exactly what setup scripts are good at, and this also gives us a
chance to dogfood them in the nextest repo proper (rather than just in the
fixtures).

This is a really nice performance improvement on my local machine (goes from
~5s to ~3s) -- should see a similar or even better result in CI.

To make the tests still work, I had to do two internal/unstable things:

1. Add a `debug current-exe` command to get the current executable. Not
part of the stable interface, though not hugely painful to stabilize it
if there's a compelling use case.
2. Add `__NEXTEST_ALT_TARGET_DIR` to ensure that we're still testing the
situation where linked paths aren't found. This is definitely a
test-only knob that shouldn't be stabilized.
  • Loading branch information
sunshowers authored Dec 10, 2024
1 parent 2635cb6 commit 337840e
Show file tree
Hide file tree
Showing 19 changed files with 718 additions and 252 deletions.
11 changes: 6 additions & 5 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ junit.store-success-output = true

[[profile.default.scripts]]
filter = 'package(integration-tests) or binary_id(nextest-runner::integration)'
setup = "cargo-fetch-fixture"
setup = "build-seed-archive"

[profile.ci]
# Don't fail fast in CI to run the full test suite.
Expand Down Expand Up @@ -57,7 +57,8 @@ max-threads = 8
[test-groups.unused-group]
max-threads = 8

[script.cargo-fetch-fixture]
command = "cargo fetch --manifest-path fixtures/nextest-tests/Cargo.toml"
capture-stdout = true
capture-stderr = true
[script.build-seed-archive]
# This command builds a seed archive that's used by the integration-tests
# package. This archive is not currently used by the older nextest-runner
# integration framework, but that should really go away at some point.
command = "cargo run --bin build-seed-archive"
40 changes: 40 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ config = { version = "0.14.1", default-features = false, features = [
chrono = "0.4.39"
clap = { version = "4.5.23", features = ["derive"] }
console-subscriber = "0.4.1"
cp_r = "0.5.2"
crossterm = { version = "0.28.1", features = ["event-stream"] }
dialoguer = "0.11.0"
debug-ignore = "1.0.5"
Expand All @@ -54,6 +55,7 @@ enable-ansi-support = "0.2.1"
# we don't use the default formatter so we don't need default features
env_logger = { version = "0.11.5", default-features = false }
fixture-data = { path = "fixture-data" }
fs-err = "3.0.0"
future-queue = "0.3.0"
futures = "0.3.31"
globset = "0.4.15"
Expand Down Expand Up @@ -124,6 +126,7 @@ tracing = "0.1.41"
tracing-subscriber = { version = "0.3.19", default-features = false, features = ["std", "tracing-log", "fmt"] }
unicode-ident = "1.0.14"
unicode-normalization = "0.1.24"
whoami = "1.5.2"
win32job = "2.0.0"
windows-sys = "0.59.0"
winnow = "0.6.20"
Expand Down
8 changes: 8 additions & 0 deletions cargo-nextest/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2099,6 +2099,9 @@ enum DebugCommand {
#[arg(value_enum)]
output_format: ExtractOutputFormat,
},

/// Print the current executable path.
CurrentExe,
}

impl DebugCommand {
Expand Down Expand Up @@ -2154,6 +2157,11 @@ impl DebugCommand {
display_output_slice(output_slice, output_format)?;
}
}
DebugCommand::CurrentExe => {
let exe = std::env::current_exe()
.map_err(|err| ExpectedError::GetCurrentExeFailed { err })?;
println!("{}", exe.display());
}
}

Ok(0)
Expand Down
10 changes: 10 additions & 0 deletions cargo-nextest/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ pub enum ReuseBuildKind {
pub enum ExpectedError {
#[error("could not change to requested directory")]
SetCurrentDirFailed { error: std::io::Error },
#[error("failed to get current executable")]
GetCurrentExeFailed {
#[source]
err: std::io::Error,
},
#[error("cargo metadata exec failed")]
CargoMetadataExecFailed {
command: String,
Expand Down Expand Up @@ -387,6 +392,7 @@ impl ExpectedError {
Self::WorkspaceRootInvalidUtf8 { .. }
| Self::WorkspaceRootInvalid { .. }
| Self::SetCurrentDirFailed { .. }
| Self::GetCurrentExeFailed { .. }
| Self::ProfileNotFound { .. }
| Self::StoreDirCreateError { .. }
| Self::RootManifestNotFound { .. }
Expand Down Expand Up @@ -455,6 +461,10 @@ impl ExpectedError {
error!("could not change to requested directory");
Some(error as &dyn Error)
}
Self::GetCurrentExeFailed { err } => {
error!("failed to get current executable");
Some(err as &dyn Error)
}
Self::CargoMetadataExecFailed { command, err } => {
error!("failed to execute `{}`", command.style(styles.bold));
Some(err as &dyn Error)
Expand Down
3 changes: 3 additions & 0 deletions fixtures/nextest-tests/.config/nextest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ default-filter = "not (test(test_flaky) | package(cdylib-example))"
platform = 'cfg(unix)'
default-filter = "not (test(test_flaky) | package(cdylib-example) | test(test_cargo_env_vars))"

[profile.archive-all]
archive.include = [{ path = "", relative-to = "target" }]

[test-groups.flaky]
max-threads = 4

Expand Down
1 change: 0 additions & 1 deletion fixtures/nextest-tests/cdylib/cdylib-link/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ extern "C" {
#[test]
fn test_multiply_two() {
assert_eq!(unsafe { multiply_two(3, 3) }, 9);

}
1 change: 1 addition & 0 deletions fixtures/nextest-tests/proc-macro-test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

2 changes: 1 addition & 1 deletion fixtures/nextest-tests/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn test_cwd() {
let cargo_toml =
std::fs::read_to_string(runtime_cwd.join("Cargo.toml")).unwrap_or_else(|error| {
panic!(
"should be able to read Cargo.toml: {}",
"error reading Cargo.toml at `{}`: {error}",
cargo_toml_path.display()
)
});
Expand Down
18 changes: 15 additions & 3 deletions integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ publish = false
name = "cargo-nextest-dup"
path = "test-helpers/cargo-nextest-dup.rs"

[[bin]]
name = "build-seed-archive"
path = "test-helpers/build-seed-archive.rs"

[dependencies]
camino.workspace = true
camino-tempfile.workspace = true

# We specify default-no-update here because if users just run:
#
# cargo build --no-default-features --features default-no-update
Expand All @@ -18,20 +25,25 @@ path = "test-helpers/cargo-nextest-dup.rs"
# We could ask distributors to always include `--package cargo-nextest` instead, but they're likely
# to forget. None of our current tests depend on self-update, so just don't include it by default.
cargo-nextest.workspace = true

color-eyre.workspace = true
clap = { workspace = true, features = ["env"] }
enable-ansi-support.workspace = true
fs-err.workspace = true
hex.workspace = true
nextest-metadata.workspace = true
nextest-workspace-hack.workspace = true
serde_json.workspace = true
sha2.workspace = true
whoami.workspace = true

[dev-dependencies]
camino-tempfile.workspace = true
camino.workspace = true
cfg-if.workspace = true
cp_r.workspace = true
fixture-data.workspace = true
insta.workspace = true
itertools.workspace = true
nextest-metadata.workspace = true
pathdiff.workspace = true
regex.workspace = true
serde_json.workspace = true
target-spec.workspace = true
27 changes: 27 additions & 0 deletions integration-tests/src/env.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) The nextest Contributors
// SPDX-License-Identifier: MIT OR Apache-2.0

#[track_caller]
pub fn set_env_vars() {
// The dynamic library tests require this flag.
std::env::set_var("RUSTFLAGS", "-C prefer-dynamic");
// Set CARGO_TERM_COLOR to never to ensure that ANSI color codes don't interfere with the
// output.
// TODO: remove this once programmatic run statuses are supported.
std::env::set_var("CARGO_TERM_COLOR", "never");
// This environment variable is required to test the #[bench] fixture. Note that THIS IS FOR
// TEST CODE ONLY. NEVER USE THIS IN PRODUCTION.
std::env::set_var("RUSTC_BOOTSTRAP", "1");

// Disable the tests which check for environment variables being set in `config.toml`, as they
// won't be in the search path when running integration tests.
std::env::set_var("__NEXTEST_NO_CHECK_CARGO_ENV_VARS", "1");

// Display empty STDOUT and STDERR lines in the output of failed tests. This
// allows tests which make sure outputs are being displayed to work.
std::env::set_var("__NEXTEST_DISPLAY_EMPTY_OUTPUTS", "1");

// Remove OUT_DIR from the environment, as it interferes with tests (some of them expect that
// OUT_DIR isn't set.)
std::env::remove_var("OUT_DIR");
}
6 changes: 6 additions & 0 deletions integration-tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright (c) The nextest Contributors
// SPDX-License-Identifier: MIT OR Apache-2.0

pub mod env;
pub mod nextest_cli;
pub mod seed;
Loading

0 comments on commit 337840e

Please sign in to comment.