Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
@@ -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'
7 changes: 7 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
30 changes: 26 additions & 4 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
17 changes: 14 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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 ?=
Expand Down Expand Up @@ -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
Expand Down
133 changes: 118 additions & 15 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
```
Comment thread
leynos marked this conversation as resolved.

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:

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions docs/snapshot-testing-in-netsuke-using-insta.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 23 additions & 2 deletions docs/whitaker-users-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`:

Expand Down
Loading
Loading