diff --git a/.config/nextest.toml b/.config/nextest.toml new file mode 100644 index 00000000..140d91cd --- /dev/null +++ b/.config/nextest.toml @@ -0,0 +1,36 @@ +# cargo-nextest configuration for Netsuke. +# +# Scope: this file governs the non-doctest pass of `make test` only. nextest +# does not execute doctests, so those run in a separate `cargo test --doc` pass +# (the `doctest` Make target). Coverage and mutation testing keep their own +# purpose-built runners and are unaffected by this file. +# +# Ownership: this configuration encodes the test-isolation contracts described +# in "Test execution" and "Test isolation utilities" in +# docs/developers-guide.md. Change it alongside that documentation. +# +# Policy: no blanket retries. A test that fails intermittently is a defect to +# diagnose, not to paper over with re-runs. Add a targeted override with a +# written rationale if a genuine external-resource constraint requires one. + +[profile.default] +# Surface hangs without failing legitimately slow suites: the documentation +# end-to-end tests shell out to real Ninja and can take tens of seconds. Warn +# after 60s; terminate a test that has run for five warning periods (300s). +slow-timeout = { period = "60s", terminate-after = 5 } + +[test-groups] +# Mutual exclusion for the integration binaries that mutate process-global +# environment state (`PATH`, `NINJA_ENV`, and ad hoc `NETSUKE_*` variables). +# Their tests are also marked `#[serial]`, which is what serialises them under +# the in-process runner used by the coverage workflow. nextest runs each test +# in its own process, so this group is not load-bearing for those existing +# tests; it exists so the serialisation contract is stated once for both +# runners and is not silently lost when a future test in these binaries reaches +# for genuinely shared state such as a fixed on-disk path. It is deliberately +# scoped to three binaries: every other test still runs fully in parallel. +serial-env = { max-threads = 1 } + +[[profile.default.overrides]] +filter = 'binary(manifest_env_tests) | binary(ninja_env_tests) | binary(env_path_tests)' +test-group = 'serial-env' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 062b1758..f7223533 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,6 +27,9 @@ jobs: # Override rust-toolchain.toml to actually use the matrix toolchain RUSTUP_TOOLCHAIN: ${{ matrix.rust }} WHITAKER_INSTALLER_VERSION: '0.2.6' + # Single source of truth for the cargo-nextest pin. `make test` runs the + # non-doctest suite through nextest, so every matrix leg needs it. + NEXTEST_VERSION: '0.9.133' steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -38,6 +41,10 @@ jobs: with: toolchain: ${{ matrix.rust }} components: rustfmt, clippy + - name: Install cargo-nextest + uses: taiki-e/install-action@18b1216eba7f8039b0f8d131d5473787f0edce68 # v2.85.3 + with: + tool: nextest@${{ env.NEXTEST_VERSION }} - name: Show rustc version run: | rustup show diff --git a/AGENTS.md b/AGENTS.md index 8c8d022b..289ccf9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -159,12 +159,16 @@ project: - `make test` executes: ```sh - cargo test --workspace + RUSTFLAGS="-D warnings" cargo nextest run --all-targets --all-features + RUSTFLAGS="-D warnings" cargo test --doc --all-features ``` - running the full workspace test suite. Use `make fmt` - (`cargo fmt --workspace`) to apply formatting fixes reported by the - formatter check. + running every unit, integration, and behavioural test through + [cargo-nextest](https://nexte.st/), then the doctests separately because + nextest cannot execute them. Both passes deny warnings, and `make test` + fails if either does. `make test-nextest` and `make doctest` run the + individual passes. Use `make fmt` (`cargo fmt --workspace`) to apply + formatting fixes reported by the formatter check. - Clippy warnings MUST be disallowed. - Fix any warnings emitted during tests in the code itself rather than silencing them. @@ -240,6 +244,24 @@ project: ### Testing +- The non-doctest suite runs under [cargo-nextest](https://nexte.st/), which + executes each test in its own process. Continuous integration (CI) pins the + version in `NEXTEST_VERSION` in `.github/workflows/ci.yml`. Read the pin from + the workflow rather than copying the number, then install that exact version + locally so local runs match CI: + + ```sh + NEXTEST_VERSION="$(sed -n "s/.*NEXTEST_VERSION: '\(.*\)'.*/\1/p" \ + .github/workflows/ci.yml)" + cargo install cargo-nextest --locked --version "$NEXTEST_VERSION" + # or, for a prebuilt binary: + cargo binstall --no-confirm "cargo-nextest@$NEXTEST_VERSION" + ``` + +- The runner is configured by `.config/nextest.toml` at the repository root. + Its scope, the narrow `serial-env` test group, and the no-blanket-retry + policy are documented under "Test execution" in `docs/developers-guide.md`. + Do not add retries to hide a flaky test; fix the test. - Use `rstest` fixtures for shared setup. - Replace duplicated tests with `#[rstest(...)]` parameterized cases. - Prefer `mockall` for ad hoc mocks/stubs. diff --git a/Makefile b/Makefile index 939267dd..3b99de58 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,14 @@ -.PHONY: help all clean test test-workflow-contracts test-typos-config build release lint lint-clippy lint-whitaker fmt check-fmt typecheck markdownlint spelling spelling-config spelling-helper-test nixie install-kani kani-check kani-full kani-ir install-verus verus formal-pr +.PHONY: help all clean test test-nextest doctest test-workflow-contracts test-typos-config build release lint lint-clippy lint-whitaker fmt check-fmt typecheck markdownlint spelling spelling-config spelling-helper-test nixie install-kani kani-check kani-full kani-ir install-verus verus formal-pr APP ?= netsuke CARGO ?= $(shell command -v cargo 2>/dev/null || printf '%s' "$$HOME/.cargo/bin/cargo") +# Extra build-parallelism flags for plain Cargo invocations, e.g. `-j 4`. BUILD_JOBS ?= +# The same concept for cargo-nextest, which spells build parallelism +# `--build-jobs N` and reserves `-j` for test concurrency. Keep the two +# variables separate so a `-j` value is never reinterpreted as a test-thread +# count. +NEXTEST_BUILD_JOBS ?= CLIPPY_FLAGS ?= --all-targets --all-features -- -D warnings KANI ?= cargo kani KANI_FLAGS ?= @@ -51,8 +57,13 @@ all: release ## Default target builds release binary clean: ## Remove build artefacts $(CARGO) clean -test: ## Run tests with warnings treated as errors - RUSTFLAGS="-D warnings" $(CARGO) test --all-targets --all-features $(BUILD_JOBS) +test: test-nextest doctest ## Run every Rust test with warnings treated as errors + +test-nextest: ## Run all non-doctest Rust tests through cargo-nextest + RUSTFLAGS="-D warnings" $(CARGO) nextest run --all-targets --all-features $(NEXTEST_BUILD_JOBS) + +doctest: ## Run doctests, which cargo-nextest cannot execute + RUSTFLAGS="-D warnings" $(CARGO) test --doc --all-features $(BUILD_JOBS) test-workflow-contracts: ## Validate the mutation-testing caller contract uv run --with 'pytest>=8' --with 'pyyaml>=6' pytest tests/workflow_contracts -q diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 858c9593..ff7cccbc 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -84,21 +84,54 @@ Run these commands before finalizing any change: - `make lint` - `make test` +`make test` runs the non-doctest suite through +[cargo-nextest](https://nexte.st/) and then runs the doctests separately. +CI pins the runner version in `NEXTEST_VERSION` in `.github/workflows/ci.yml`. +Install that same version locally so local runs match CI; read the pin from the +workflow rather than copying the number, so the two cannot drift: + +```bash +NEXTEST_VERSION="$(sed -n "s/.*NEXTEST_VERSION: '\(.*\)'.*/\1/p" \ + .github/workflows/ci.yml)" +cargo install cargo-nextest --locked --version "$NEXTEST_VERSION" +# or, for a prebuilt binary: +cargo binstall --no-confirm "cargo-nextest@$NEXTEST_VERSION" +``` + +See [Test execution](#test-execution) for what the checked-in nextest +configuration does and does not cover. + `make lint` runs rustdoc with warnings denied, `cargo clippy`, and the [Whitaker](whitaker-users-guide.md) Dylint suite (`whitaker --all -- --all-targets --all-features`). Install Whitaker through the standalone installer described in the [Whitaker user's guide](whitaker-users-guide.md) so local linting matches continuous integration (CI); `make lint-clippy` runs the Clippy-only subset. -Whitaker is configured by `dylint.toml` at the repository root. The -`no_std_fs_operations` lint currently ignores in-source `allow`/`expect` -attributes, so `dylint.toml` excludes each sanctioned ambient-filesystem scope -with a documented rationale: the `build_script_build` Cargo build-script crate, -the `netsuke` application crate, the `test_support` test-fixture crate, and the -enumerated integration-test and workflow-contract crates. Netsuke itself needs -ambient access for executable discovery through `PATH`, cross-directory symlink -canonicalization, and temporary-file synchronization; other filesystem access -should remain capability-scoped. +Whitaker is configured by `dylint.toml` at the repository root, where each +sanctioned ambient-filesystem scope for `no_std_fs_operations` carries a +documented rationale. + +Prefer `excluded_paths` over `excluded_crates`: a path entry exempts one module +and its descendants, whereas a crate entry exempts a whole compilation unit. +The application crate is scoped this way — only +`netsuke::stdlib::which::lookup` (executable discovery through `PATH` and +cross-directory symlink canonicalization, which `cap_std` cannot express) and +`netsuke::runner::process::file_io` (temporary-file synchronization) are +exempt; the rest of `netsuke` stays under the capability policy. The +behavioural step definitions, CLI integration tests, and shared +workflow-reading helper that stage fixtures ambiently are scoped the same way. +A crate-level entry is justified only when the ambient access lives in the +crate root itself, where a path entry would be no narrower — that covers the +Cargo build script, the `test_support` fixture crate, and the enumerated +integration-test crates. + +Permanent exceptions belong in `dylint.toml`, scoped as narrowly as the lint +allows. The lint does honour in-source lint attributes, but this repository +denies `clippy::allow_attributes`, so `#[allow(no_std_fs_operations)]` will not +compile here; an in-source exemption must be a *temporary*, item-level +`#[expect(no_std_fs_operations, reason = "…")]` that states the reason and the +route back to compliance. Prefer migrating to `cap_std` over any of these; +reach for an exclusion only when the operation is irreducibly ambient. When command output is long, preserve exit codes and logs: @@ -442,6 +475,71 @@ artefacts: the job uses a Kani-specific cache key derived from `tools/kani/VERSION` and the Makefile, then caches the job-local Kani Cargo home plus Kani support-file home. +## Test execution + +`make test` is the canonical entry point and composes two passes: + +- `make test-nextest` — + `cargo nextest run --all-targets --all-features`, with + `RUSTFLAGS="-D warnings"`. This runs every unit, integration, `rstest`, and + `rstest-bdd` test. +- `make doctest` — `cargo test --doc --all-features`, with the same + `RUSTFLAGS`. nextest cannot execute doctests, so they need their own pass. + Note that the previous `cargo test --all-targets` invocation never ran + doctests either; the separate target is what makes a broken documentation + example fail the gate. + +If either pass fails, `make test` fails. Run the individual targets when +iterating, but treat `make test` as the gate. + +Cargo spells build parallelism `-j`; nextest reserves `-j` for test +concurrency and spells build parallelism `--build-jobs`. The Makefile +therefore keeps `BUILD_JOBS` (Cargo flags) and `NEXTEST_BUILD_JOBS` (nextest +flags) as separate variables rather than reinterpreting one as the other. + +### nextest configuration + +The runner is configured by `.config/nextest.toml` at the workspace root. It +governs the non-doctest pass only, and deliberately stays small: + +- **`serial-env` test group** (`max-threads = 1`) covering exactly three + binaries: `manifest_env_tests`, `ninja_env_tests`, and `env_path_tests`. + These mutate process-global environment state — `PATH`, `NINJA_ENV`, and ad + hoc `NETSUKE_*` variables. Every other test remains fully parallel. +- **No blanket retries.** A test that fails intermittently is a defect to + diagnose. Add a targeted override with a written rationale only when a + genuine external-resource constraint requires one. +- **A conservative slow timeout** (warn after 60s, terminate after five + warning periods) so a hung test surfaces without failing the legitimately + slow documentation end-to-end suites, which shell out to real Ninja. + +### How this relates to `#[serial]` and the isolation utilities + +nextest runs each test in its own process, so environment and +working-directory mutations cannot leak between tests the way they can under +the threaded in-process harness. The `EnvLock`, `EnvVarGuard`, and `CwdGuard` +utilities described in [Test isolation utilities](#test-isolation-utilities), +and the `#[serial]` markers on the tests in the three binaries above, remain +necessary because the coverage workflow still drives an in-process runner. + +The `serial-env` group is therefore not load-bearing for the tests that exist +today; it states the serialization contract once so both runners agree, and so +it is not silently lost if a future test in those binaries reaches for +genuinely shared state such as a fixed on-disk path. + +### Runners not covered by this configuration + +- **Coverage** (`.github/workflows/coverage-main.yml`, and the coverage step in + `ci.yml`) delegates to the `generate-coverage` shared action, which drives + its own `cargo llvm-cov` invocation. It does not call `make test` and is + unaffected by `.config/nextest.toml`. +- **Mutation testing** (`.github/workflows/mutation-testing.yml`) calls the + shared `mutation-cargo.yml` reusable workflow with `--all-features`, matching + the feature set `make test` uses. Its runner is owned by that workflow. + +Changing either to use nextest is a deliberate decision, not something that +should follow implicitly from this file. + ## Test suite map Netsuke uses a mixed strategy: @@ -474,7 +572,6 @@ packaged crate builds successfully for release. It then uses build-script sources, including `build_l10n_audit.rs`, and rejects stale `ninja_env/` paths. - ### Temporary executable test helpers The low-level executable-stub primitive is owned by @@ -574,10 +671,16 @@ use the root crate's development dependency. ## Behavioural testing strategy -Behavioural tests run through `cargo test` using `rstest-bdd`, not a bespoke -runner. The `scenarios!` macro in `tests/bdd_tests.rs` discovers feature files -and binds a shared fixture entry point (`world: TestWorld`) to each generated -scenario test. +Behavioural tests use `rstest-bdd`, not a bespoke runner, and are executed by +cargo-nextest alongside every other test (see +[Test execution](#test-execution)). The `scenarios!` macro in +`tests/bdd_tests.rs` discovers feature files and binds a shared fixture entry +point (`world: TestWorld`) to each generated scenario test. + +nextest runs each generated scenario in its own process. That reinforces the +per-scenario isolation policy below rather than conflicting with it: scenario +state cannot leak across process boundaries, so the policy's requirement to +recreate state per test is enforced by the runner as well as by convention. ### State and isolation policy @@ -634,7 +737,7 @@ These points are strategy rules, not optional style guidance. 3. Reuse existing fixtures/helpers before adding new world state. 4. Add typed parameter wrappers in `tests/bdd/types.rs` when step arguments represent distinct domain concepts. -5. Run `cargo test --test bdd_tests` and then the full quality gates. +5. Run `cargo nextest run --test bdd_tests` and then the full quality gates. ## Manifest `foreach` expansion diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md index 99554db1..775a7863 100644 --- a/docs/snapshot-testing-in-netsuke-using-insta.md +++ b/docs/snapshot-testing-in-netsuke-using-insta.md @@ -278,6 +278,13 @@ function, serializing locale state across the test suite. ## Running and Updating Snapshot Tests +> In this repository the canonical runner is cargo-nextest: `make test`, or +> `cargo nextest run --test ninja_snapshot_tests` for a focused run. See +> [Test execution](developers-guide.md#test-execution). `cargo insta` needs to +> be told which runner to drive, so use `cargo insta test --test-runner +> nextest` and `cargo insta review` when accepting changes. The `cargo test` +> invocations below are the generic form. + To execute the snapshot tests, run `cargo test`. All tests (including our new snapshot tests) will run. On the first run (or whenever a snapshot differs from expectations), test failures will indicate snapshot changes. diff --git a/docs/whitaker-users-guide.md b/docs/whitaker-users-guide.md index 69f08d0e..4c1f112d 100644 --- a/docs/whitaker-users-guide.md +++ b/docs/whitaker-users-guide.md @@ -620,6 +620,7 @@ Enforces capability-based filesystem access by forbidding direct use of ```toml [no_std_fs_operations] excluded_crates = ["my_cli_entrypoint", "my_test_utilities"] +excluded_paths = ["my_app::legacy_io", "my_app::bin::migrate"] ``` The `excluded_crates` option allows specified crates to use `std::fs` @@ -629,8 +630,28 @@ operations without triggering diagnostics. This is useful for: - Test support utilities that manage fixtures with ambient access - Build scripts or code generators that require direct filesystem operations -> **Note:** Use Rust crate names (underscores), not Cargo package names -> (hyphens). For example, use `my_cli_app` rather than `my-cli-app`. +The `excluded_paths` option narrows an exclusion to individual modules rather +than a whole crate. Each entry is a fully qualified path anchored at the crate +identifier, and it matches on segment boundaries: `my_app::legacy_io` exempts +that module and everything nested beneath it (for example +`my_app::legacy_io::reader`), but never a sibling such as +`my_app::legacy_io_utils`. Reach for this when only a bounded corner of a crate +needs ambient filesystem access, while the rest stays under the capability +policy. + +> **Note:** For both options, use Rust identifiers (underscores), not Cargo +> package names (hyphens). For example, use `my_cli_app` rather than +> `my-cli-app`, and `my_app::legacy_io` rather than `my-app::legacy_io`. +> +> **Tip:** For an ad hoc, single-site exemption that travels with the code, an +> item-level lint attribute also works, since the lint honours Rust's +> lint-level attributes. +> +> **Netsuke deviation from upstream — preserve when re-importing this guide.** +> This repository denies `clippy::allow_attributes`, so +> `#[allow(no_std_fs_operations)]` will not compile here. Use a narrowly scoped +> `excluded_paths` entry in `dylint.toml` for a permanent exemption, or a +> temporary `#[expect(no_std_fs_operations, reason = "…")]` on the item. **How to fix:** Replace `std::fs` with `cap_std`: diff --git a/dylint.toml b/dylint.toml index 726c02c9..4fdb9676 100644 --- a/dylint.toml +++ b/dylint.toml @@ -1,32 +1,68 @@ # Whitaker lint configuration. # -# Crates excluded from the no_std_fs_operations lint because they legitimately -# require std::fs access: +# `no_std_fs_operations` enforces capability-based filesystem access. Where an +# exclusion is genuinely warranted, prefer `excluded_paths` — it exempts one +# module and its descendants — over `excluded_crates`, which exempts an entire +# compilation unit. A crate-level entry is justified only when the ambient +# access lives in the crate root itself, where a path entry would be identical +# to a crate entry. See docs/whitaker-users-guide.md. +[no_std_fs_operations] + +# Modules that need ambient filesystem access, scoped as narrowly as the lint +# allows. Everything else in these crates stays under the capability policy. +excluded_paths = [ + # Application boundary. `netsuke` probes executables in directories supplied + # by the ambient PATH and follows cross-directory symlinks when + # canonicalising them; `cap_std` refuses symlinks that leave the directory, + # so these operations cannot be capability-scoped. + "netsuke::stdlib::which::lookup", + # Syncs the ambient temporary file the runner writes its Ninja file to. + "netsuke::runner::process::file_io", + + # Behavioural step definitions that stage scenario fixtures — manifests, + # workspaces, and captured output — through ambient tempdir access. + "bdd_tests::bdd::steps::advanced_usage", + "bdd_tests::bdd::steps::conditional_manifest", + "bdd_tests::bdd::steps::fs", + "bdd_tests::bdd::steps::manifest_command", + "bdd_tests::bdd::steps::process", + "bdd_tests::bdd::steps::progress_output", + "bdd_tests::bdd::steps::stdlib::assertions", + "bdd_tests::bdd::steps::stdlib::workspace", + + # CLI integration tests that stage configuration files in ambient tempdirs. + "cli_tests::cli_tests::config_discovery", + "cli_tests::cli_tests::config_selection", + "cli_tests::cli_tests::merge", + + # The shared workflow-reading helper (`tests/common/mod.rs`), included by + # each workflow contract crate. The crates themselves stay under policy. + "workflow_ci::common", + "workflow_release::common", +] + +# Whole crates whose ambient filesystem access lives in the crate root, so a +# path entry would be no narrower than a crate entry: # - build_script_build: the Cargo build script writes generated man pages and # audit artefacts to ambient paths supplied by Cargo (`OUT_DIR`, `target/`), # where capability-based handles offer no benefit. -# - netsuke: the application crate deliberately performs executable probes in -# directories supplied by ambient PATH, follows cross-directory symlinks when -# canonicalising those executables, and syncs its ambient temporary file. -# These application-boundary operations cannot be capability-scoped. # - test_support: the shared test-fixture crate; fixture management uses # ambient tempdir access by design (see test_support/src/fs.rs), matching -# the "test support utilities" exclusion in the Whitaker user's guide. +# the "test support utilities" exclusion in the Whitaker user's guide. The +# crate is currently excluded from the workspace, so Whitaker does not lint +# it; the entry is kept so folding it in does not silently reopen the +# question. # - integration test crates (tests/*.rs): fixture management writes manifests, # workspaces, and captured output with ambient tempdir access, which the -# Whitaker user's guide lists as a sanctioned exclusion. Add new test crates -# here deliberately when they need ambient fixture I/O. -[no_std_fs_operations] +# Whitaker user's guide lists as a sanctioned exclusion. +# +# Add a new entry here only when the ambient I/O really is at the crate root; +# otherwise add a path above, and prefer migrating to `cap_std` over either. excluded_crates = [ "build_script_build", - "netsuke", "test_support", "advanced_usage_tests", "assert_cmd_tests", - "bdd_tests", - "cli_tests", - "dependabot_config_tests", - "logging_stderr_tests", "manifest_glob_tests", "ninja_snapshot_tests", "novice_flow_smoke_tests", @@ -39,7 +75,5 @@ excluded_crates = [ "stdlib_which_tests", "which_diagnostic_snapshot_tests", "workflow_build_and_package", - "workflow_ci", - "workflow_release", "workflow_shared_actions_pins", ] diff --git a/tests/makefile_test_target.rs b/tests/makefile_test_target.rs new file mode 100644 index 00000000..afa07481 --- /dev/null +++ b/tests/makefile_test_target.rs @@ -0,0 +1,194 @@ +//! Contract tests for the canonical `make test` entry point. +//! +//! `make test` is the single command local development and continuous +//! integration (CI) both run. These tests pin the runner contract it encodes: +//! non-doctest tests go through cargo-nextest, doctests run separately because +//! nextest cannot execute them, and both passes deny warnings. They also assert +//! that the checked-in nextest configuration still declares the narrow +//! serialisation group the environment-mutating suites depend on. + +use anyhow::{Context, Result, ensure}; +use camino::Utf8Path; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use toml::Value; + +/// Opens the repository root as a capability-scoped directory handle. +/// +/// Every read in this file is relative to that handle, so the tests cannot +/// reach outside the checkout. +fn repo_root() -> Result { + Dir::open_ambient_dir(env!("CARGO_MANIFEST_DIR"), ambient_authority()) + .context("open the repository root as a capability-scoped directory") +} + +fn read_repo_file(relative: &Utf8Path) -> Result { + repo_root()? + .read_to_string(relative) + .with_context(|| format!("{relative} should be readable")) +} + +/// Splits a Make rule line into its target and its prerequisites. +/// +/// Trailing `## ` help comments are discarded so `help` annotations do not leak +/// into the prerequisite list. +fn parse_rule(line: &str) -> Option<(&str, Vec<&str>)> { + if line.starts_with(['\t', ' ', '#', '.']) { + return None; + } + let (target, rest) = line.split_once(':')?; + if target.is_empty() || rest.starts_with('=') { + return None; + } + let prerequisites = rest + .split("##") + .next() + .unwrap_or_default() + .split_whitespace() + .collect(); + Some((target.trim(), prerequisites)) +} + +/// Returns the prerequisites declared for `target`. +fn target_prerequisites(contents: &str, target: &str) -> Option> { + contents.lines().find_map(|line| { + let (name, prerequisites) = parse_rule(line)?; + (name == target).then(|| prerequisites.into_iter().map(ToOwned::to_owned).collect()) + }) +} + +/// Returns the tab-indented recipe lines for `target`, joined by newlines. +fn target_recipe(contents: &str, target: &str) -> Option { + let mut lines = contents + .lines() + .skip_while(|line| parse_rule(line).is_none_or(|(name, _)| name != target)); + lines.next()?; + let recipe: Vec<&str> = lines + .take_while(|line| line.starts_with('\t') || line.trim().is_empty()) + .filter(|line| line.starts_with('\t')) + .collect(); + Some(recipe.join("\n")) +} + +#[test] +fn unit_parses_make_rules_and_ignores_help_comments() { + assert_eq!( + parse_rule("test: test-nextest doctest ## Run every Rust test"), + Some(("test", vec!["test-nextest", "doctest"])) + ); + assert_eq!( + parse_rule("doctest: ## Run doctests"), + Some(("doctest", vec![])) + ); + assert_eq!(parse_rule("BUILD_JOBS ?="), None); + assert_eq!(parse_rule("\tcargo nextest run"), None); + assert_eq!(parse_rule(".PHONY: test doctest"), None); +} + +#[test] +fn unit_extracts_recipe_lines_for_a_target() { + let makefile = "test: doctest ## composite\n\ndoctest: ## docs\n\tcargo test --doc\n\techo done\n\nother:\n\ttrue\n"; + + assert_eq!(target_recipe(makefile, "test").as_deref(), Some("")); + assert_eq!( + target_recipe(makefile, "doctest").as_deref(), + Some("\tcargo test --doc\n\techo done") + ); + assert_eq!(target_recipe(makefile, "missing"), None); +} + +#[test] +fn behavioural_make_test_composes_the_nextest_and_doctest_passes() -> Result<()> { + let makefile = read_repo_file(Utf8Path::new("Makefile"))?; + + let prerequisites = + target_prerequisites(&makefile, "test").context("Makefile should declare a test target")?; + for expected in ["test-nextest", "doctest"] { + ensure!( + prerequisites.iter().any(|name| name == expected), + "make test should depend on {expected}, found {prerequisites:?}" + ); + } + + let nextest_recipe = target_recipe(&makefile, "test-nextest") + .context("Makefile should declare a test-nextest target")?; + ensure!( + nextest_recipe.contains("nextest run"), + "test-nextest should route through cargo nextest run, found {nextest_recipe:?}" + ); + ensure!( + nextest_recipe.contains("--all-targets"), + "test-nextest should cover every test target, found {nextest_recipe:?}" + ); + ensure!( + nextest_recipe.contains("--all-features"), + "test-nextest should enable all features, found {nextest_recipe:?}" + ); + ensure!( + nextest_recipe.contains(r#"RUSTFLAGS="-D warnings""#), + "test-nextest should deny warnings, found {nextest_recipe:?}" + ); + + let doctest_recipe = + target_recipe(&makefile, "doctest").context("Makefile should declare a doctest target")?; + ensure!( + doctest_recipe.contains("--doc"), + "doctest should invoke the doctest harness, found {doctest_recipe:?}" + ); + ensure!( + !doctest_recipe.contains("nextest"), + "doctests cannot run under nextest, found {doctest_recipe:?}" + ); + ensure!( + doctest_recipe.contains(r#"RUSTFLAGS="-D warnings""#), + "doctest should deny warnings, found {doctest_recipe:?}" + ); + Ok(()) +} + +/// Returns the `[[profile.default.overrides]]` entries. +fn profile_overrides(config: &Value) -> Option<&[Value]> { + config + .get("profile")? + .get("default")? + .get("overrides")? + .as_array() + .map(Vec::as_slice) +} + +/// Returns the `max-threads` declared for a named test group. +fn test_group_max_threads(config: &Value, group: &str) -> Option { + config + .get("test-groups")? + .get(group)? + .get("max-threads")? + .as_integer() +} + +#[test] +fn behavioural_nextest_config_binds_the_env_binaries_to_the_serial_env_group() -> Result<()> { + let config: Value = read_repo_file(&Utf8Path::new(".config").join("nextest.toml"))? + .parse() + .context("nextest configuration should be valid TOML")?; + + let max_threads = test_group_max_threads(&config, "serial-env") + .context("nextest configuration should declare the serial-env test group")?; + ensure!( + max_threads == 1, + "serial-env should serialize its members, found max-threads = {max_threads}" + ); + + let overrides = + profile_overrides(&config).context("the default profile should declare overrides")?; + let filter = overrides + .iter() + .filter(|entry| entry.get("test-group").and_then(Value::as_str) == Some("serial-env")) + .find_map(|entry| entry.get("filter").and_then(Value::as_str)) + .context("an override should assign the serial-env group with a filter")?; + for binary in ["manifest_env_tests", "ninja_env_tests", "env_path_tests"] { + ensure!( + filter.contains(&format!("binary({binary})")), + "the serial-env override should cover {binary}, found filter {filter:?}" + ); + } + Ok(()) +} diff --git a/tests/workflow_ci.rs b/tests/workflow_ci.rs index 51d18fe6..0425d9ee 100644 --- a/tests/workflow_ci.rs +++ b/tests/workflow_ci.rs @@ -68,6 +68,147 @@ fn step_name<'a>(step: &'a Value, expected_name: &str) -> Option<&'a Mapping> { (name == expected_name).then_some(mapping) } +fn named_step<'a>(steps: &'a [Value], name: &str) -> Result<&'a Mapping> { + steps + .iter() + .find_map(|step| step_name(step, name)) + .with_context(|| format!("job should include the {name} step")) +} + +fn step_index(steps: &[Value], name: &str) -> Result { + steps + .iter() + .position(|step| step_name(step, name).is_some()) + .with_context(|| format!("job should include the {name} step")) +} + +fn step_input(step: &Mapping, key: YamlKey) -> Option<&str> { + mapping_get(step, YamlKey("with")) + .and_then(Value::as_mapping) + .and_then(|with| mapping_get(with, key)) + .and_then(Value::as_str) +} + +fn job_env(job: &Mapping, key: YamlKey) -> Option<&str> { + mapping_get(job, YamlKey("env")) + .and_then(Value::as_mapping) + .and_then(|env| mapping_get(env, key)) + .and_then(Value::as_str) +} + +/// Returns true when `reference` is `@<40-character lowercase-hex SHA>`. +/// +/// Dependabot owns the SHA value, so contract tests assert the shape of the +/// pin rather than a literal commit; see "Workflow pins and Dependabot" in +/// `docs/developers-guide.md`. +fn is_pinned_action_ref(reference: &str, action: &str) -> bool { + let Some(pin) = reference + .strip_prefix(action) + .and_then(|rest| rest.strip_prefix('@')) + else { + return false; + }; + pin.len() == 40 + && pin + .bytes() + .all(|byte| byte.is_ascii_digit() || matches!(byte, b'a'..=b'f')) +} + +/// Returns true when `version` is an exact `major.minor.patch` pin. +fn is_exact_version(version: &str) -> bool { + let mut parts = version.split('.'); + let numeric = |component: Option<&str>| { + component.is_some_and(|value| { + !value.is_empty() && value.bytes().all(|byte| byte.is_ascii_digit()) + }) + }; + numeric(parts.next()) + && numeric(parts.next()) + && numeric(parts.next()) + && parts.next().is_none() +} + +#[test] +fn unit_recognizes_pinned_action_refs() { + assert!(is_pinned_action_ref( + "taiki-e/install-action@0123456789abcdef0123456789abcdef01234567", + "taiki-e/install-action" + )); + assert!(!is_pinned_action_ref( + "taiki-e/install-action@v2", + "taiki-e/install-action" + )); + assert!(!is_pinned_action_ref( + "taiki-e/install-action@0123456789ABCDEF0123456789ABCDEF01234567", + "taiki-e/install-action" + )); + assert!(!is_pinned_action_ref( + "someone-else/install-action@0123456789abcdef0123456789abcdef01234567", + "taiki-e/install-action" + )); +} + +#[test] +fn unit_recognizes_exact_versions() { + assert!(is_exact_version("0.9.133")); + assert!(!is_exact_version("0.9")); + assert!(!is_exact_version("0.9.133.1")); + assert!(!is_exact_version("latest")); + assert!(!is_exact_version("^0.9.133")); +} + +#[test] +fn behavioural_ci_workflow_installs_pinned_cargo_nextest() -> Result<()> { + let contents = workflow_contents("ci.yml").expect("CI workflow should be readable"); + let workflow: Value = serde_yaml::from_str(&contents).context("parse CI workflow YAML")?; + let build_test = job(&workflow, "build-test")?; + + let version = job_env(build_test, YamlKey("NEXTEST_VERSION")) + .context("build-test job should pin NEXTEST_VERSION")?; + ensure!( + is_exact_version(version), + "NEXTEST_VERSION should pin an exact version, found {version:?}" + ); + + let steps = steps(build_test)?; + let install = named_step(steps, "Install cargo-nextest")?; + let uses = mapping_get(install, YamlKey("uses")) + .and_then(Value::as_str) + .context("Install cargo-nextest step should reference an action")?; + ensure!( + is_pinned_action_ref(uses, "taiki-e/install-action"), + "cargo-nextest installer should be pinned to a full commit SHA, found {uses:?}" + ); + ensure!( + step_input(install, YamlKey("tool")) == Some("nextest@${{ env.NEXTEST_VERSION }}"), + "cargo-nextest installer should resolve its pin from NEXTEST_VERSION" + ); + ensure!( + !install.contains_key(Value::String("if".to_owned())), + "cargo-nextest should install on every matrix leg because every leg runs make test" + ); + Ok(()) +} + +#[test] +fn behavioural_ci_workflow_runs_tests_through_the_make_target() -> Result<()> { + let contents = workflow_contents("ci.yml").expect("CI workflow should be readable"); + let workflow: Value = serde_yaml::from_str(&contents).context("parse CI workflow YAML")?; + let build_test = job(&workflow, "build-test")?; + let steps = steps(build_test)?; + + let test_step = named_step(steps, "Test")?; + ensure!( + mapping_get(test_step, YamlKey("run")).and_then(Value::as_str) == Some("make test"), + "the Test step should run the canonical make target" + ); + ensure!( + step_index(steps, "Install cargo-nextest")? < step_index(steps, "Test")?, + "cargo-nextest should be installed before make test runs" + ); + Ok(()) +} + #[test] fn behavioural_ci_workflow_wires_kani_smoke_job() -> Result<()> { let contents = workflow_contents("ci.yml").expect("CI workflow should be readable");