Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d3ecf37
docs(users-guide): add advanced usage chapter with workflows and exam…
leynos Apr 7, 2026
bc2342a
feat(advanced-usage): add advanced usage chapter with BDD tests and f…
leynos Apr 7, 2026
b46cef8
refactor(tests): clean up BDD step definitions and manifest command code
leynos Apr 8, 2026
5c893c4
feat(tests,bdd): refactor environment variable handling and add tests
leynos Apr 8, 2026
2024fb3
refactor(tests): improve environment variable management in BDD tests
leynos Apr 9, 2026
2ce14e9
test(bdd): enhance manifest command tests and environment variable fo…
leynos Apr 10, 2026
b56778f
Update code
leynos Apr 10, 2026
7d64213
test(advanced-usage): add integration tests for advanced usage workflows
leynos Apr 11, 2026
bbf2f8e
feat(bdd): forward scenario environment vars to child netsuke processes
leynos Apr 13, 2026
f3d4ed6
refactor(bdd): eliminate environment variable data races in BDD tests
leynos Apr 14, 2026
ac556d1
feat(tests,bdd,env): add advanced usage tests and env isolation for m…
leynos Apr 15, 2026
5ed9327
docs(testing): enhance docs and tests for env isolation and advanced …
leynos Apr 16, 2026
91ce45b
refactor(tests): eliminate duplication in config layering tests
leynos Apr 16, 2026
240785f
refactor(bdd,test_support): add non-consuming accessor to NinjaEnvGuard
leynos Apr 16, 2026
f1955b7
refactor(tests,test_support): fix PATH forwarding and consolidate con…
leynos Apr 16, 2026
20b930a
test(advanced_usage): add tests for manifest output separation and pa…
leynos Apr 17, 2026
89a0ecf
refactor(tests): use isolated execution and complete validation in ad…
leynos Apr 17, 2026
804aec9
docs(testing): add BDD test execution flow sequence diagram
leynos Apr 17, 2026
02874ce
refactor(tests,docs): tighten diagnostics, add case names, and clarif…
leynos Apr 17, 2026
b50a2a9
chore(ci): update shared-actions to 6b13b519 and improve docs readabi…
leynos Apr 17, 2026
864bb0f
docs(developers-guide): document test infrastructure APIs from PR #283
leynos Apr 18, 2026
a8da1cf
refactor(tests): fix environment handling in run_netsuke_with_env
leynos Apr 18, 2026
d1529c0
chore(ci): update WiX Toolset from version 6 to 7
leynos Apr 18, 2026
c90f68d
docs(developers-guide): fix punctuation and spelling
leynos Apr 18, 2026
23b0991
docs(developers-guide): correct track_env_var example and guard metho…
leynos Apr 18, 2026
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
12 changes: 6 additions & 6 deletions .github/workflows/build-and-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ name: Build and Package
wix-extension-version:
description: WiX Toolset extension version for Windows packaging
required: false
default: '6'
default: '7'
Comment thread
leynos marked this conversation as resolved.
type: string

jobs:
Expand All @@ -79,14 +79,14 @@ jobs:
python-version: ${{ inputs['python-version'] }}

- name: Build release binary
uses: leynos/shared-actions/.github/actions/rust-build-release@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/rust-build-release@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
target: ${{ inputs.target }}
bin-name: ${{ inputs['bin-name'] }}

- name: Package Linux artefacts with dependencies
if: inputs.platform == 'linux'
uses: leynos/shared-actions/.github/actions/linux-packages@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/linux-packages@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
project-dir: .
package-name: ${{ inputs['bin-name'] }}
Expand All @@ -108,7 +108,7 @@ jobs:
if: inputs.platform != 'linux'
id: stage
uses: >-
leynos/shared-actions/.github/actions/stage-release-artefacts@7f4cc57326d14b55f7eea300e0454653636fb6a1
leynos/shared-actions/.github/actions/stage-release-artefacts@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
config-file: .github/release-staging.toml
target: ${{ inputs['stage-target'] }}
Expand All @@ -130,7 +130,7 @@ jobs:
- name: Build Windows installer package
if: inputs.platform == 'windows'
id: package_windows
uses: leynos/shared-actions/.github/actions/windows-package@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/windows-package@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
product-name: ${{ inputs['bin-name'] }}
manufacturer: Leynos
Expand All @@ -148,7 +148,7 @@ jobs:
- name: Build macOS installer package
if: inputs.platform == 'macos'
id: package_macos
uses: leynos/shared-actions/.github/actions/macos-package@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/macos-package@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
name: ${{ inputs['bin-name'] }}
identifier: com.leynos.${{ inputs['bin-name'] }}
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- name: Setup Rust
uses: leynos/shared-actions/.github/actions/setup-rust@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/setup-rust@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
toolchain: ${{ matrix.rust }}
components: rustfmt, clippy
Expand All @@ -48,15 +48,15 @@ jobs:
run: make test
- name: Test and Measure Coverage
if: matrix.rust == 'stable'
uses: leynos/shared-actions/.github/actions/generate-coverage@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/generate-coverage@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
output-path: lcov.info
format: lcov
- name: Upload coverage data to CodeScene
env:
CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }}
if: matrix.rust == 'stable' && env.CS_ACCESS_TOKEN != ''
uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
format: lcov
access-token: ${{ env.CS_ACCESS_TOKEN }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/netsukefile-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
- name: Checkout repository
uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- name: Setup Rust
uses: leynos/shared-actions/.github/actions/setup-rust@7f4cc57326d14b55f7eea300e0454653636fb6a1
uses: leynos/shared-actions/.github/actions/setup-rust@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
toolchain: ${{ matrix.rust }}
- name: Show rustc version
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ name: Release Binary
the WiX CLI.
required: false
type: string
default: '6'
default: '7'
outputs:
bin_name:
description: Binary name extracted from Cargo.toml
Expand Down Expand Up @@ -76,11 +76,11 @@ jobs:
- id: release_modes
name: Determine release modes
uses: >-
leynos/shared-actions/.github/actions/determine-release-modes@7f4cc57326d14b55f7eea300e0454653636fb6a1
leynos/shared-actions/.github/actions/determine-release-modes@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
- id: ensure_version
name: Read package version from Cargo.toml
uses: >-
leynos/shared-actions/.github/actions/ensure-cargo-version@7f4cc57326d14b55f7eea300e0454653636fb6a1
leynos/shared-actions/.github/actions/ensure-cargo-version@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
check-tag: ${{ fromJSON(steps.release_modes.outputs['should-publish']) }}
- id: wix_extension_version
Expand All @@ -94,7 +94,7 @@ jobs:
"$GITHUB_EVENT_PATH")"
fi
if [ -z "$version" ] || [ "$version" = 'null' ]; then
version='6'
version='7'
fi
echo "value=$version" >>"$GITHUB_OUTPUT"
- id: manifest_path
Expand All @@ -107,7 +107,7 @@ jobs:
- id: bin_name
name: Extract binary name from Cargo.toml
uses: >-
leynos/shared-actions/.github/actions/export-cargo-metadata@7f4cc57326d14b55f7eea300e0454653636fb6a1
leynos/shared-actions/.github/actions/export-cargo-metadata@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
manifest-path: ${{ steps.manifest_path.outputs.value }}
fields: bin-name
Expand Down Expand Up @@ -221,7 +221,7 @@ jobs:
- id: upload_assets
name: Upload artefacts to release
uses: >-
leynos/shared-actions/.github/actions/upload-release-assets@7f4cc57326d14b55f7eea300e0454653636fb6a1
leynos/shared-actions/.github/actions/upload-release-assets@6b13b519f99c5b461be8cc21ecf19c2ec5907b9c
with:
release-tag: ${{ github.ref_name }}
bin-name: ${{ needs.metadata.outputs.bin_name }}
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ mockall = "0.11"
camino = "1.2.0"
test_support = { path = "test_support" }
strip-ansi-escapes = "0.2"
toml = "0.8"

# Target-specific dev-deps
[target.'cfg(unix)'.dev-dependencies]
Expand Down
175 changes: 167 additions & 8 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,33 @@ let _guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH");
For BDD steps that need to track mutations through `TestWorld`, use
`mutate_env_var` from `tests/bdd/helpers/env_mutation.rs` instead.

### `original_ref()` on environment guards

`NinjaEnvGuard` and `EnvGuard<E>` both expose a non-consuming accessor:

```rust
pub fn original_ref(&self) -> Option<&OsString>
```

Use this to inspect the value that was in the environment *before* the guard
was activated, without consuming the guard. This is the correct way for BDD
steps to obtain the prior value when calling `track_env_var` because the
consuming `into_original(self)` would drop the guard prematurely:

Comment thread
coderabbitai[bot] marked this conversation as resolved.
```rust
let guard = override_ninja_env(&SystemEnv::new(), &ninja_path);
let previous = guard.original_ref().cloned();
world.track_env_var(
ninja_env::NINJA_ENV.to_owned(),
previous,
Some(ninja_path.as_os_str().to_owned()),
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
world.ninja_env_guard = Some(guard);
```

The consuming `into_original(self) -> Option<OsString>` method remains
available when the guard is no longer needed after the read.

### `CwdGuard`

Tests that call `std::env::set_current_dir` must restore the original working
Expand All @@ -158,9 +185,8 @@ let _cwd_guard = CwdGuard::acquire()?;
std::env::set_current_dir(temp.path())?;
```

Acquire `EnvLock` and then `CwdGuard` so Rust drops them in reverse
declaration order: `CwdGuard` restores the CWD first, and `EnvLock` releases
second.
Acquire `EnvLock` and then `CwdGuard` so Rust drops them in reverse declaration
order: `CwdGuard` restores the CWD first, and `EnvLock` releases second.

### `restore_many` and `restore_many_locked`

Expand Down Expand Up @@ -244,12 +270,14 @@ Table: Scenario state groups and fields
| Localization state | `localization_lock`, `localization_guard`, `locale_config`, `locale_env`, `locale_cli_override`, `locale_system`, `resolved_locale`, `locale_message` | Scenario-level localizer overrides and resolution state. |
| HTTP server state | `http_server`, `stdlib_url` | Test HTTP server fixture for fetch scenarios. |
| Output state | `output_mode`, `simulated_no_color`, `simulated_term`, `output_prefs`, `simulated_no_emoji`, `rendered_prefix` | Accessibility and output preference resolution. |
| Environment state | `env_vars`, `env_lock`, `original_cwd` | Environment variable snapshots, scenario-scoped lock, and CWD capture. |
| Environment state | `env_vars`, `env_vars_forward`, `env_lock`, `original_cwd` | Restoration snapshot, forwarding map, scenario lock, and CWD capture. |

### Key `TestWorld` methods

- `track_env_var(key, previous)` — record a variable for restoration at
scenario end.
- `track_env_var(key, previous, new_value)` — record a variable for
restoration at scenario end and store `new_value` in `env_vars_forward` so
that `build_netsuke_command` can forward it to child processes without
re-reading the process environment.
- `ensure_env_lock()` — acquire the scenario-scoped `EnvLock` on first
call; subsequent calls are no-ops. Also captures the current working
directory for later restoration.
Expand Down Expand Up @@ -354,12 +382,143 @@ Versioning and compatibility rules:
- `cli_overrides_from_matches` must continue to emit a JSON object, even when
no CLI override is present.
- `is_empty_value` treats only the empty object `{}` as "no CLI overrides".
Downstream tooling must not replace an empty object with `null`, `[]`, or
any other sentinel.
Downstream tooling must not replace an empty object with `null`, `[]`, or any
other sentinel.
- Additional properties are ignored by `diag_json` resolution and may be
present because the same layer object also participates in full config
merging.

## BDD command helpers and environment handling

The BDD step module `tests/bdd/steps/manifest_command.rs` provides three
helpers that launch the netsuke binary in a controlled environment:

- **`netsuke_executable()`** — locates the compiled netsuke binary using
`assert_cmd::cargo::cargo_bin!("netsuke")`. Returns the resolved `PathBuf` or
an error if the binary is not found.
- **`build_netsuke_command(world, args)`** — constructs an
`assert_cmd::Command` with a sanitized environment. The helper:
1. Calls `env_clear()` to strip the inherited environment for test
isolation.
2. Forwards `PATH` (via `std::env::var_os`) **without** acquiring `EnvLock`
because the calling thread may already hold the lock via a
`NinjaEnvGuard` stored on the `TestWorld` — and `std::sync::Mutex` is
not reentrant. The direct read is safe: when a `NinjaEnvGuard` is
alive, it serializes all env mutations; when no guard is alive, the
`PATH` mutation from `prepend_dir_to_path` has already completed.
3. Forwards all scenario-tracked environment variables from
`world.env_vars_forward` (including `NETSUKE_NINJA` and any variables set
by BDD steps) without reading the process environment, eliminating data
races.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- **`run_netsuke_and_store(world, args)`** — calls `build_netsuke_command`,
runs the command, and stores stdout, stderr, and exit status in the
`TestWorld` fixture for subsequent `Then` step assertions.

### Environment contract

After `env_clear()`, only these variables are present in the spawned command:

| Variable | Source | Purpose |
| ------------ | ------------------------ | ----------------------------- |
| `PATH` | Host `std::env::var_os` | Locate ninja and subprocesses |
| Scenario env | `world.env_vars_forward` | BDD-step-configured overrides |

`world.env_vars_forward` is a `HashMap<String, OsString>` containing the
*current* values that BDD steps intend to pass to child processes, including
`NETSUKE_NINJA` when a fake ninja is installed. The helper iterates
`env_vars_forward` and calls `cmd.env(key, value)` for each entry, so the child
process receives exactly the variables that steps have configured without
reading the process environment.

The separate `world.env_vars` map is a **restoration snapshot**: keys are
variables set during the scenario, and values are their *previous* values (for
restoration when the scenario ends). It is not used by `build_netsuke_command`.

### `given_config_file_with_setting` step (`tests/bdd/steps/advanced_usage.rs`)

The Gherkin step `a workspace with config file setting {key} to {value}` writes
a `.netsuke.toml` file to the scenario's temp directory with the given key set
to a TOML value derived from `{value}`:

- `"true"` and `"false"` are parsed as TOML booleans.
- All other values are written as TOML strings.

This step uses the `toml = "0.8"` dev-dependency added to `Cargo.toml` for
serialization. Do not add further crate dependencies to support this step; the
existing `toml` crate is sufficient for key/value configuration files of this
Comment thread
coderabbitai[bot] marked this conversation as resolved.
kind. The step is intentionally limited to scalar types: extend it only when a
concrete BDD scenario requires numeric or array values.

### BDD test execution flow (e2e behavioural tests)

The following diagram illustrates how a BDD scenario flows through the test
infrastructure, from scenario invocation through workspace setup, command
execution, and assertion validation. This applies to **end-to-end behavioural
tests** defined in Gherkin feature files, not unit or code-level integration
tests:

```mermaid
sequenceDiagram
actor Developer
participant BddRunner
participant TestWorld
participant AdvancedUsageSteps
participant ManifestCommandSteps
participant AssertCmdCommand
participant NetsukeBinary
participant NinjaTool

Developer->>BddRunner: run bdd_tests advanced_usage
BddRunner->>TestWorld: create TestWorld fixture

BddRunner->>AdvancedUsageSteps: execute Given a minimal Netsuke workspace
AdvancedUsageSteps->>ManifestCommandSteps: reuse workspace_setup_steps
ManifestCommandSteps->>TestWorld: create_workspace_with_manifest()

BddRunner->>AdvancedUsageSteps: execute When netsuke is run with args "manifest -"
AdvancedUsageSteps->>TestWorld: set_env_from_world()
TestWorld->>AssertCmdCommand: build_command_with_explicit_path()
AssertCmdCommand->>AssertCmdCommand: inherit_NINJA_ENV()
AssertCmdCommand->>AssertCmdCommand: apply_world_environment_overrides()
AssertCmdCommand->>NetsukeBinary: spawn_with_env_and_path()
NetsukeBinary->>NinjaTool: optional_ninja_invocation()
NinjaTool-->>NetsukeBinary: build_status
NetsukeBinary-->>AssertCmdCommand: exit_code_stdout_stderr
AssertCmdCommand-->>TestWorld: store_process_output()

BddRunner->>AdvancedUsageSteps: execute Then stdout should contain Ninja_manifest
AdvancedUsageSteps->>TestWorld: assert_stdout_contains_manifest_markers()

BddRunner->>AdvancedUsageSteps: execute And stderr should be empty
AdvancedUsageSteps->>TestWorld: assert_stderr_empty()

BddRunner-->>Developer: scenario_passes
```

**Figure**: End-to-end BDD test execution sequence showing how workspace setup,
environment isolation, command invocation, and assertions flow through the test
infrastructure. The `TestWorld` fixture coordinates state across steps, while
`build_netsuke_command` ensures environment isolation via `env_clear()` and
explicit forwarding of scenario-configured variables. This flow applies to
feature-file-based behavioural tests, not code-level unit or integration tests.

### Integration test helper

`test_support::netsuke::run_netsuke_in(current_dir, args)` provides a simpler
interface for integration tests outside the BDD framework. It sets `PATH` to an
empty string (relying on the resolved binary path) but does **not** call
`env_clear()`, so other environment variables (including `NETSUKE_NINJA` set
via `override_ninja_env`) are inherited normally.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

For tests that need **deterministic, isolated** child-process environments, use
`test_support::netsuke::run_netsuke_in_with_env(current_dir, args, extra_env)`.
Unlike `run_netsuke_in`, this variant calls `env_clear()` so the child inherits
**only** the variables supplied in `extra_env`, plus two automatically
forwarded variables: `PATH` (from the host `std::env::var_os`) and
`NETSUKE_NINJA` (forwarded when an `override_ninja_env` guard is active in the
current process). Use this helper for configuration-layering tests or any test
that sets environment variables which could race with parallel test execution.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
## Documentation upkeep

When test strategy or behavioural test usage changes, update this file in the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,19 @@ overridden by `NETSUKE_COLOUR_POLICY=auto`, and further overridden by

- Risk: environment variable mutations in BDD tests can deadlock or become
flaky if `EnvLock` is not held for the entire scenario lifetime. Severity:
high Likelihood: medium Mitigation: use
`tests/bdd/helpers/env_mutation::mutate_env_var()` for mutating environment
variables instead of raw `std::env::set_var` calls. `mutate_env_var()`
acquires the scenario `EnvLock` internally, so no explicit lock call is
required. Cleanup happens automatically via the `TestWorld` drop path, which
restores all mutated variables.
high Likelihood: medium Mitigation: use the wrappers in `test_support::env`
instead of raw `std::env::set_var` calls. `test_support::env::set_var()`
acquires `EnvLock` internally for one-off updates, `VarGuard` gives RAII-safe
scoped restoration, and `remove_var()` mirrors the same pattern for unsets.
When a scenario requires batched cleanup, collect the original values and
restore them with `restore_many()` from `TestWorld::drop`, or keep
`VarGuard`s alive for the scenario lifetime. For BDD scenarios specifically,
`tests/bdd/helpers/env_mutation::mutate_env_var()` provides a convenient
wrapper that acquires the scenario `EnvLock` internally and tracks variables
for automatic cleanup via `TestWorld::drop`. Replace any examples that
reference `std::env::set_var` directly with `test_support::env::set_var` or
`mutate_env_var`, and show `VarGuard` for scoped restores so `EnvLock` is
always respected.

- Risk: `build.rs` symbol anchoring may be missed for new shared helpers,
causing `make lint` failures. Severity: medium Likelihood: high Mitigation:
Expand Down
Loading
Loading