Skip to content

Commit

Permalink
[nextest-runner] archive and reuse stdlibs
Browse files Browse the repository at this point in the history
We need stdlibs in two cases:

1. For proc-macro tests, the host stdlib.
2. For -C prefer-dynamic, the target stdlib.

Archive and reuse them, if available.

Also add tests for running nextest without rustup, including archives. This
should be a pretty comprehensive test.
  • Loading branch information
sunshowers committed May 23, 2024
1 parent 21dd0f0 commit 2f113ef
Show file tree
Hide file tree
Showing 18 changed files with 479 additions and 79 deletions.
90 changes: 89 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,22 @@ jobs:
NEXTEST_DOUBLE_SPAWN: 0
run: cargo local-nt run --profile ci

- name: Test without rustup wrappers
env:
CARGO_NEXTEST: target/debug/cargo-nextest
RUSTUP_AVAILABLE: 1
shell: bash
run: ./scripts/nextest-without-rustup.sh run --profile ci

- name: Upload nextest binary
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
with:
name: cargo-nextest-${{ matrix.os }}-${{ matrix.rust-version }}
path: |
target/debug/cargo-nextest
target/debug/cargo-nextest.exe
if-no-files-found: error

test-archive-target-runner:
name: Test archives with a target runner
# gcc-mingw-w64-x86-64-win32 requires Ubuntu 22.04
Expand Down Expand Up @@ -128,11 +144,83 @@ jobs:
- name: Build cargo-nextest
run: cargo build --package cargo-nextest
- name: Archive test fixtures
env:
CARGO_NEXTEST: target/debug/cargo-nextest
RUSTUP_AVAILABLE: 1
run: |
cargo local-nt archive --manifest-path fixtures/nextest-tests/Cargo.toml \
./scripts/nextest-without-rustup.sh archive --manifest-path fixtures/nextest-tests/Cargo.toml \
--target x86_64-pc-windows-gnu --archive-file target/fixture-archive.tar.zst \
--package cdylib-example --package nextest-derive
# This ensures that for target binaries, we always use the libdir in the archive, never the
# one installed by rustup.
- name: Remove x86_64-pc-windows-gnu target from rustup
run: rustup target remove x86_64-pc-windows-gnu

- name: Run test fixtures
env:
CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUNNER: wine
run: cargo local-nt run --archive-file target/fixture-archive.tar.zst

# Completely uninstall rustup -- this is similar to running an archive on a machine without
# cargo.
- name: Uninstall rustup
run: rustup self uninstall -y

- name: Run test fixtures without rustup wrappers
env:
CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUNNER: wine
CARGO_NEXTEST: target/debug/cargo-nextest
run: ./scripts/nextest-without-rustup.sh run --archive-file target/fixture-archive.tar.zst

# Upload the archive for use in the next job.
- name: Upload archive
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
with:
name: fixture-archive
path: target/fixture-archive.tar.zst
if-no-files-found: error

test-archive-runner-dest:
name: Test archives with a runner (destination)
strategy:
matrix:
os:
- ubuntu-latest
- windows-latest
runs-on: ${{ matrix.os }}
needs:
- test-archive-target-runner
- build
steps:
- uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4
- name: Download nextest binary
uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
with:
name: cargo-nextest-${{ matrix.os }}-stable
path: nextest-bin
- name: Download archive
uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
with:
name: fixture-archive
path: fixture-archive
- name: Run test fixtures (host)
if: ${{ matrix.os == 'ubuntu-latest' }}
env:
CARGO_NEXTEST: nextest-bin/cargo-nextest
run: |
chmod +x $CARGO_NEXTEST
./scripts/nextest-without-rustup.sh run \
--archive-file fixture-archive/fixture-archive.tar.zst \
--workspace-remap $GITHUB_WORKSPACE/fixtures/nextest-tests \
-E 'platform(host)'
- name: Run test fixtures (target)
if: ${{ matrix.os == 'windows-latest' }}
shell: bash
env:
CARGO_NEXTEST: nextest-bin/cargo-nextest.exe
run: |
./scripts/nextest-without-rustup.sh run \
--archive-file fixture-archive/fixture-archive.tar.zst \
--workspace-remap $GITHUB_WORKSPACE/fixtures/nextest-tests \
-E 'platform(target)'
1 change: 1 addition & 0 deletions cargo-nextest/src/reuse_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ pub(crate) fn make_path_mapper(
info.workspace_remap(),
orig_target_dir,
info.target_dir_remap(),
info.libdir_mapper.clone(),
)
.map_err(|err| {
let arg_name = match err.kind() {
Expand Down
5 changes: 4 additions & 1 deletion fixtures/nextest-tests/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,11 @@ fn test_cargo_env_vars() {
Ok("process-per-test"),
"NEXTEST_EXECUTION_MODE set to process-per-test"
);

// https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates
assert_env!("CARGO");

// Note: we do not test CARGO here because nextest does not set it -- it's set by Cargo when
// invoked as `cargo nextest`.
assert_env!(
"CARGO_MANIFEST_DIR",
"__NEXTEST_ORIGINAL_CARGO_MANIFEST_DIR"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: integration-tests/tests/integration/main.rs
expression: output.stderr_as_str()
---
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, and 7 extra paths to <archive-file>
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, 7 extra paths, and 1 standard library to <archive-file>
Warning ignoring extra path `<target-dir>/excluded-dir` because it does not exist
Warning ignoring extra path `<target-dir>/depth-0-dir` specified with depth 0 since it is a directory
Warning ignoring extra path `<target-dir>/file_that_does_not_exist.txt` because it does not exist
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: integration-tests/tests/integration/main.rs
expression: output.stderr_as_str()
---
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, and 7 extra paths to <archive-file>
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, 7 extra paths, and 1 standard library to <archive-file>
Warning ignoring extra path `<target-dir>/excluded-dir` because it does not exist
Warning ignoring extra path `<target-dir>/depth-0-dir` specified with depth 0 since it is a directory
Warning ignoring extra path `<target-dir>/file_that_does_not_exist.txt` because it does not exist
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: integration-tests/tests/integration/main.rs
expression: output.stderr_as_str()
---
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, and 1 extra path to <archive-file>
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, 1 extra path, and 1 standard library to <archive-file>
error: error creating archive `<archive-file>`

Caused by:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: integration-tests/tests/integration/main.rs
expression: output.stderr_as_str()
---
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, and 2 linked paths to <archive-file>
Archiving 17 binaries (including 2 non-test binaries), 2 build script output directories, 2 linked paths, and 1 standard library to <archive-file>
Warning linked path `<target-dir>/debug/build/<cdylib-link-hash>/does-not-exist` not found, requested by: cdylib-link v0.1.0
(this is a bug in this crate that should be fixed)
Archived <file-count> files to <archive-file> in <duration>
4 changes: 4 additions & 0 deletions nextest-metadata/src/test_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,10 @@ impl PlatformLibdirUnavailable {
/// version of nextest.
pub const OLD_SUMMARY: Self = Self::new_const("old-summary");

/// The libdir is unavailable because a build was reused from an archive, and the libdir was not
/// present in the archive
pub const NOT_IN_ARCHIVE: Self = Self::new_const("not-in-archive");

/// Converts a static string into Self.
pub const fn new_const(reason: &'static str) -> Self {
Self(Cow::Borrowed(reason))
Expand Down
8 changes: 8 additions & 0 deletions nextest-runner/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ pub(crate) mod plural {
"these crates"
}
}

pub(crate) fn libraries_str(count: usize) -> &'static str {
if count == 1 {
"library"
} else {
"libraries"
}
}
}

/// Write out a test name.
Expand Down
12 changes: 8 additions & 4 deletions nextest-runner/src/list/rust_build_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl RustBuildMeta<BinaryListState> {
build_script_out_dirs: self.build_script_out_dirs.clone(),
linked_paths: self.linked_paths.clone(),
state: PhantomData,
build_platforms: self.build_platforms.clone(),
build_platforms: self.build_platforms.map_libdir(path_mapper.libdir_mapper()),
}
}
}
Expand Down Expand Up @@ -103,6 +103,11 @@ impl RustBuildMeta<TestListState> {
/// These paths are prepended to the dynamic library environment variable for the current
/// platform (e.g. `LD_LIBRARY_PATH` on non-Apple Unix platforms).
pub fn dylib_paths(&self) -> Vec<Utf8PathBuf> {
// Add rust libdirs to the path if available, so we can run test binaries that depend on
// libstd.
//
// We could be smarter here and only add the host libdir for host binaries and the target
// libdir for target binaries, but it's simpler to just add both for now.
let libdirs = self
.build_platforms
.host
Expand All @@ -115,11 +120,12 @@ impl RustBuildMeta<TestListState> {
.as_ref()
.and_then(|target| target.libdir.as_path()),
)
.cloned()
.map(|libdir| libdir.to_path_buf())
.collect::<Vec<_>>();
if libdirs.is_empty() {
log::warn!("failed to detect the rustc libdir, may fail to list or run tests");
}

// Cargo puts linked paths before base output directories.
self.linked_paths
.keys()
Expand All @@ -138,8 +144,6 @@ impl RustBuildMeta<TestListState> {
// This is the order paths are added in by Cargo.
[with_deps, abs_base]
}))
// Add the rustc libdir paths to the search paths to run procudure macro binaries. See
// details in https://github.com/nextest-rs/nextest/issues/1493.
.chain(libdirs)
.unique()
.collect()
Expand Down
8 changes: 8 additions & 0 deletions nextest-runner/src/list/test_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ impl<'g> TestList<'g> {
let stream = futures::stream::iter(test_artifacts).map(|test_binary| {
async {
if filter.should_obtain_test_list_from_binary(&test_binary) {
log::debug!(
"executing test binary to obtain test list: {}",
test_binary.binary_id,
);
// Run the binary to obtain the test list.
let (non_ignored, ignored) = test_binary.exec(&lctx, ctx.target_runner).await?;
let (bin, info) = Self::process_output(
Expand All @@ -245,6 +249,10 @@ impl<'g> TestList<'g> {
Ok::<_, CreateTestListError>((bin, info))
} else {
// Skipped means no tests, so test_count doesn't need to be modified.
log::debug!(
"skipping test binary because of filters: {}",
test_binary.binary_id,
);
Ok(Self::process_skipped(test_binary))
}
}
Expand Down
30 changes: 28 additions & 2 deletions nextest-runner/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
use crate::{
cargo_config::{CargoTargetArg, TargetTriple},
errors::{RustBuildMetaParseError, TargetTripleError, UnknownHostPlatform},
reuse_build::{LibdirMapper, PlatformLibdirMapper},
};
use camino::Utf8PathBuf;
use camino::{Utf8Path, Utf8PathBuf};
use nextest_metadata::{
BuildPlatformsSummary, HostPlatformSummary, PlatformLibdirSummary, PlatformLibdirUnavailable,
TargetPlatformSummary,
Expand Down Expand Up @@ -40,6 +41,17 @@ impl BuildPlatforms {
})
}

/// Maps libdir paths.
pub fn map_libdir(&self, mapper: &LibdirMapper) -> Self {
Self {
host: self.host.map_libdir(&mapper.host),
target: self
.target
.as_ref()
.map(|target| target.map_libdir(&mapper.target)),
}
}

/// Returns the argument to pass into `cargo metadata --filter-platform <triple>`.
pub fn to_cargo_target_arg(&self) -> Result<CargoTargetArg, TargetTripleError> {
match &self.target {
Expand Down Expand Up @@ -192,6 +204,13 @@ impl HostPlatform {
libdir: PlatformLibdir::from_summary(summary.libdir),
})
}

fn map_libdir(&self, mapper: &PlatformLibdirMapper) -> Self {
Self {
platform: self.platform.clone(),
libdir: mapper.map(&self.libdir),
}
}
}

/// The target platform.
Expand Down Expand Up @@ -227,6 +246,13 @@ impl TargetPlatform {
libdir: PlatformLibdir::from_summary(summary.libdir),
})
}

fn map_libdir(&self, mapper: &PlatformLibdirMapper) -> Self {
Self {
triple: self.triple.clone(),
libdir: mapper.map(&self.libdir),
}
}
}

/// A platform libdir.
Expand Down Expand Up @@ -292,7 +318,7 @@ impl PlatformLibdir {
}

/// Returns self as a path if available.
pub fn as_path(&self) -> Option<&Utf8PathBuf> {
pub fn as_path(&self) -> Option<&Utf8Path> {
match self {
Self::Available(path) => Some(path),
Self::Unavailable(_) => None,
Expand Down
Loading

0 comments on commit 2f113ef

Please sign in to comment.