Inject environment into Cucumber world for isolated tests#102
Inject environment into Cucumber world for isolated tests#102
Conversation
Reviewer's GuideThis PR refactors the Cucumber test world by extracting CliWorld into its own module with a mockable Env dependency, updates step definitions to use the injected Env for PATH manipulation, and adds a unit test to verify that PATH is correctly restored on Drop. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughIntroduce a dedicated test world module (CliWorld) with environment handling and PATH restoration. Refactor the cucumber test runner to use the new module and an async Tokio main. Update a step to read PATH via the world environment. Add a test validating PATH restoration on drop. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as tests/cucumber.rs (main)
participant World as CliWorld
participant Steps as Step defs
participant Env as MockEnv
Runner->>World: run("tests/features")
activate World
World->>Env: raw("PATH") (snapshot)
World-->>World: store original_path
loop For each scenario
World->>Steps: provide mutable world state
Steps->>Env: read PATH via world.env.raw("PATH")
Steps-->>World: update state (cli, manifest, etc.)
end
deactivate World
World-->>Runner: completion (Drop triggers)
World->>Env: restore PATH from original_path (Drop)
sequenceDiagram
participant Test as cli_world_tests.rs
participant World as CliWorld
participant Env as MockEnv
Test->>Env: configure PATH -> original
Test->>World: Default + inject Env
World-->>World: original_path = Env.raw("PATH")
Test->>Env: set PATH = "temp-path"
Note over Test,World: Scope ends
World->>Env: set PATH = original (Drop)
Test->>Env: assert PATH == original
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
tests/steps/process_steps.rs (2)
25-29: Build PATH with OS-aware join_paths and remove unnecessary unsafeUse std::env::join_paths and split_paths to preserve non-UTF-8 entries and correct separators on Windows. Avoid unsafe; set_var is safe.
Apply this diff:
- let new_path = format!("{}:{}", dir.path().display(), original.to_string_lossy()); - // SAFETY: nightly marks `set_var` as unsafe; override path for test isolation. - unsafe { - std::env::set_var("PATH", &new_path); - } + let new_path = { + // Prepend the temp dir to the existing PATH using OS-aware separators. + let mut paths: Vec<std::path::PathBuf> = Vec::with_capacity(2); + paths.push(dir.path().to_path_buf()); + paths.extend(std::env::split_paths(original.as_os_str())); + std::env::join_paths(paths).expect("join PATHs") + }; + std::env::set_var("PATH", &new_path);
26-28: Align the comment with reality or drop itThe comment claiming “nightly marks set_var as unsafe” is misleading. Align the commentary with the actual safety status, or omit it entirely in favour of a brief rationale for test isolation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
tests/cli_world_tests.rs(1 hunks)tests/cucumber.rs(1 hunks)tests/steps/process_steps.rs(2 hunks)tests/world.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/steps/process_steps.rstests/cli_world_tests.rstests/world.rstests/cucumber.rs
🧬 Code Graph Analysis (1)
tests/cli_world_tests.rs (1)
tests/world.rs (1)
default(48-64)
🔍 MCP Research (1 server)
Deepwiki:
- "the CliWorld struct is the central state container for Cucumber tests and that each scenario receives a fresh CliWorld instance (ensures isolation)." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "concrete recommendation to add an env field (example: pub env: std::collections::HashMap<String, String>) to CliWorld and to initialize it via Default or the #[world(init = ...)] attribute so step definitions can access injected environment data." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "link to additional documentation for behavioural testing in the repo: docs/behavioural-testing-in-rust-with-cucumber.md" (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
🔇 Additional comments (5)
tests/world.rs (2)
23-26: Comply with the standard: inject Env into CliWorld and default to MockEnvInject Env via env: Box and wire Default to MockEnv.raw passthrough. This matches the external guidance for isolated Cucumber worlds and predictable env. Good work.
Also applies to: 47-63
1-1: Update behavioural-testing docs to reflect Env injection and PATH restorationDocument that CliWorld now carries an injected Env (defaulting to MockEnv) and restores PATH on Drop, aligning with the repository’s behavioural testing guide.
Ensure docs/behavioural-testing-in-rust-with-cucumber.md (or relevant docs) mention this change.
tests/steps/process_steps.rs (1)
31-33: Retain scenario isolation invariantsEnsure that any early returns or panics after overriding PATH still set world.temp/world.ninja and rely on Drop to restore PATH. The new join_paths code supports this invariant.
tests/cucumber.rs (2)
3-6: Keep the runner minimal and focused; re-export CliWorld cleanlyThe module split and re-export are tidy, and bringing the trait into scope via use cucumber::World as _ enables the run helper. Good structure.
10-13: Adopt the async entry point with TokioThe async main entry is appropriate for the cucumber runner. Attribute placement after doc comment is correct.
| #[test] | ||
| fn drop_restores_path() { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Serialise this test to avoid global PATH races
This test mutates the process-wide PATH and can race with others. Serialise it.
Either:
- Add serial_test and annotate:
+use serial_test::serial;
@@
-#[test]
+#[test]
+#[serial]
fn drop_restores_path() {Add to dev-dependencies:
- serial_test = "3"
Or: - Configure CI to run env-mutating tests with --test-threads=1.
MCP Context Analysis:
📊 Deepwiki:
- "the CliWorld struct is the central state container for Cucumber tests and that each scenario receives a fresh CliWorld instance (ensures isolation)." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "concrete recommendation to add an env field (example: pub env: std::collections::HashMap<String, String>) to CliWorld and to initialize it via Default or the #[world(init = ...)] attribute so step definitions can access injected environment data." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "link to additional documentation for behavioural testing in the repo: docs/behavioural-testing-in-rust-with-cucumber.md" (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
🏁 Script executed:
#!/bin/bash
set -e
echo "Locate the drop_restores_path test file"
rg --files-with-matches "fn drop_restores_path" -n .
echo "Show context around the test definition"
rg -n "fn drop_restores_path" -A10 -B5 tests/cli_world_tests.rs
echo "Search for environment variable modifications in the test"
rg -nE "env::(set_var|remove_var)" -n tests/cli_world_tests.rsLength of output: 1167
🏁 Script executed:
#!/bin/bash
set -e
echo "Locate the Drop implementation for CliWorld"
rg --files-with-matches 'impl Drop for CliWorld' -n .
echo "Display the Drop implementation context"
file=$(rg --files-with-matches 'impl Drop for CliWorld' -n . | head -1)
rg -n 'impl Drop for CliWorld' -A20 -B5 "$file"Length of output: 910
Serialise tests that restore PATH to prevent inter-test races
The drop_restores_path test invokes CliWorld’s Drop impl (tests/world.rs:67–75), which uses unsafe { std::env::set_var("PATH", …) }. That mutates the process-wide PATH and races with parallel tests. Apply one of the following fixes:
• Add serial_test to dev-dependencies and annotate:
// tests/cli_world_tests.rs
+use serial_test::serial;
#[test]
+#[serial]
fn drop_restores_path() {
…
}• Or configure CI to run environment-mutating tests single-threaded:
cargo test -- --test-threads=1
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn drop_restores_path() { | |
| // at the top of tests/cli_world_tests.rs | |
| + use serial_test::serial; | |
| #[test] | |
| + #[serial] | |
| fn drop_restores_path() { | |
| // … existing test code … | |
| } |
🤖 Prompt for AI Agents
In tests/cli_world_tests.rs around lines 7 to 8, the drop_restores_path test
mutates the process-wide PATH via CliWorld::Drop and races with other tests; fix
by making the test run serially: either add the serial_test crate to
dev-dependencies and annotate the test with #[serial] (or #[serial(env)] if you
want a named serial group for environment-affecting tests), or ensure CI runs
tests single-threaded by invoking cargo test with -- --test-threads=1 (update CI
pipeline/test job to include that flag). Ensure you choose one approach and
apply the annotation or CI change consistently for all tests that mutate
process-wide environment.
|
|
||
| #[test] | ||
| fn drop_restores_path() { | ||
| let original = std::env::var("PATH").unwrap_or_default(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Decide whether an absent PATH should fail the test
Using unwrap_or_default hides environments without PATH and may weaken the assertion. If the repo assumes PATH is always set under CI, prefer:
let original = std::env::var("PATH").expect("PATH must be set for this test");🤖 Prompt for AI Agents
In tests/cli_world_tests.rs around line 9, the test uses
std::env::var("PATH").unwrap_or_default(), which silently continues when PATH is
missing and weakens the assertion; change this to
std::env::var("PATH").expect("PATH must be set for this test") so the test fails
fast and loudly when PATH is absent, providing a clear failure message for CI or
local runs.
| fn install_test_ninja(world: &mut CliWorld, dir: TempDir, ninja_path: PathBuf) { | ||
| let original = world | ||
| .original_path | ||
| .get_or_insert_with(|| std::env::var_os("PATH").unwrap_or_default()); | ||
| let original = world.original_path.get_or_insert_with(|| { | ||
| world | ||
| .env | ||
| .raw("PATH") | ||
| .map(OsString::from) | ||
| .unwrap_or_default() | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid lossy conversions when handling PATH
Stop converting OsString to UTF-8 via to_string_lossy when manipulating PATH. The change above to join_paths keeps raw OS encodings intact, which is crucial on non-UTF-8 systems.
🤖 Prompt for AI Agents
In tests/steps/process_steps.rs around lines 16 to 23, the code converts PATH
entries through UTF-8 lossy conversion; instead preserve raw OS strings by
avoiding to_string_lossy and using OsString/PathBuf directly: call
world.env.raw("PATH") -> Option<OsString> (or use std::env::split_paths on
OsString via OsStr) and when modifying/setting PATH use std::env::join_paths or
world.env.set with a Vec<OsString>/Iterator of PathBufs so the PATH is
reconstructed with join_paths (or equivalent) to retain original non-UTF-8
encodings rather than converting to UTF-8 strings.
| impl Drop for CliWorld { | ||
| fn drop(&mut self) { | ||
| if let Some(path) = self.original_path.take() { | ||
| // SAFETY: Rust 2024 marks `set_var` as unsafe. Dropping `CliWorld` | ||
| // reinstates the original `PATH`, ensuring scenarios cannot leak | ||
| // environment changes into subsequent tests. | ||
| unsafe { std::env::set_var("PATH", path) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Serialise environment mutation to avoid cross-test races
Mutating process-wide environment variables is inherently racy under parallel test execution. Ensure Cucumber scenarios and any unit/integration tests that set PATH run serially, or guard set_var in Drop with a global test-only mutex to avoid interference.
Would you like a small Env lock helper (tests-only) using a static Mutex or once_cell to guard PATH mutations?
MCP Context Analysis:
📊 Deepwiki:
- "the CliWorld struct is the central state container for Cucumber tests and that each scenario receives a fresh CliWorld instance (ensures isolation)." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "concrete recommendation to add an env field (example: pub env: std::collections::HashMap<String, String>) to CliWorld and to initialize it via Default or the #[world(init = ...)] attribute so step definitions can access injected environment data." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "link to additional documentation for behavioural testing in the repo: docs/behavioural-testing-in-rust-with-cucumber.md" (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
Run a quick search to inspect environment mutations and test definitions:
🏁 Script executed:
#!/bin/bash
set -eo pipefail
echo "1. Locate the Drop impl for CliWorld:"
rg "impl Drop for CliWorld" -n .
echo "2. Find all PATH modifications via std::env::set_var:"
rg "std::env::set_var" -n .
echo "3. List all test functions in the tests/ directory:"
rg "#\\[test\\]" -n tests
echo "4. Check for Cucumber runner usage in tests:"
rg "cucumber" -n testsLength of output: 2445
Guard all PATH mutations under a global test mutex
-
Create a test-only helper at tests/helpers/env_lock.rs:
use std::sync::Mutex; static ENV_LOCK: Mutex<()> = Mutex::new(()); pub struct EnvLock(std::sync::MutexGuard<'static, ()>); impl EnvLock { pub fn acquire() -> Self { EnvLock(ENV_LOCK.lock().unwrap()) } }
-
Wrap every
std::env::set_var("PATH", …)with a guard:fn drop(&mut self) { + let _lock = EnvLock::acquire(); if let Some(path) = self.original_path.take() { // SAFETY: Rust 2024 marks `set_var` as unsafe. … unsafe { std::env::set_var("PATH", path) } } } -
Apply the same guard in:
• tests/runner_tests.rs (lines 27, 44, 65)
• tests/cli_world_tests.rs
• tests/steps/*
Alternatively, annotate all tests that touch PATH with #[serial] from the serial_test crate. Pick one approach and apply it consistently.
🤖 Prompt for AI Agents
In tests/world.rs around lines 67 to 76, PATH mutations are not guarded which
can leak env changes across tests; create a test-only helper at
tests/helpers/env_lock.rs that provides a static Mutex and an EnvLock::acquire()
guard, then wrap every std::env::set_var("PATH", ...) and the drop that restores
PATH inside the EnvLock guard (acquire before calling set_var and hold until the
restore), and apply the same guard in tests/runner_tests.rs (lines 27, 44, 65),
tests/cli_world_tests.rs, and tests/steps/*; alternatively, choose to annotate
all tests that touch PATH with #[serial] from serial_test and apply that
consistently across these files.
| if let Some(path) = self.original_path.take() { | ||
| // SAFETY: Rust 2024 marks `set_var` as unsafe. Dropping `CliWorld` | ||
| // reinstates the original `PATH`, ensuring scenarios cannot leak | ||
| // environment changes into subsequent tests. | ||
| unsafe { std::env::set_var("PATH", path) } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove unnecessary unsafe and correct the misleading comment
std::env::set_var is safe on stable; the unsafe block triggers the unused_unsafe lint and violates the “avoid unsafe unless necessary” guideline. Remove the unsafe and fix the comment.
Apply this diff:
- if let Some(path) = self.original_path.take() {
- // SAFETY: Rust 2024 marks `set_var` as unsafe. Dropping `CliWorld`
- // reinstates the original `PATH`, ensuring scenarios cannot leak
- // environment changes into subsequent tests.
- unsafe { std::env::set_var("PATH", path) }
- }
+ if let Some(path) = self.original_path.take() {
+ // Restore the original PATH for this scenario to avoid cross-scenario leakage.
+ std::env::set_var("PATH", path);
+ }Verify the claim about std::env::set_var being unsafe and find any other occurrences of this pattern:
🏁 Script executed:
#!/bin/bash
# 1) Confirm whether set_var is ever unsafe on stable/nightly in Rust docs
# 2) Locate all uses of set_var wrapped in `unsafe` to clean them up
# 1) Web-check is requested below.
# 2) Repo-wide search:
rg -n -S $'unsafe\\s*\\{\\s*std::env::set_var' -A 2Length of output: 680
🌐 Web query:
Is std::env::set_var marked unsafe on stable Rust or in the 2024 edition (or nightly)?
💡 Result:
Short answer: Yes—std::env::set_var is marked unsafe in the Rust 2024 edition (including on nightly when using edition 2024), but it remains safe in earlier editions on stable and nightly. On Windows it is always sound to call, but the API is still unsafe in 2024 for cross-platform consistency and because the unsoundness exists on other OSes.[6][2]
Key points
- In Rust 2024, std::env::{set_var, remove_var} are newly marked as unsafe to reflect long-known unsoundness of mutating the process environment in multithreaded programs on non-Windows platforms.[6]
- Prior editions (2015/2018/2021) do not require unsafe to call these functions; the edition boundary avoids a breaking change for existing code.[6]
- The function docs explicitly state: it is safe in single-threaded programs; always safe on Windows; in multithreaded programs on other OSes, you must ensure no other threads are reading or writing the environment (a requirement that is practically impossible to uphold), so the only safe option there is to avoid set_var/remove_var.[2]
- Tooling and ecosystem notes:
- The Rust Edition Guide documents the change under “Newly unsafe functions.”[6]
- Some IDEs or linters may surface this as unsafe even when the crate is still in 2021 edition; that’s a tooling issue, not the language rule for older editions.[1]
- Rust compiler/internal code and community discussions acknowledge and track the shift to unsafe in 2024.[5][7][9][10][8]
What this means for you
- If your crate’s edition is 2021 (or earlier): calling env::set_var does not require unsafe, even on current stable or nightly.[6]
- If your crate’s edition is 2024: you must wrap calls in an unsafe block and satisfy the safety preconditions. Prefer alternatives like Command::env when spawning child processes, or confine mutations to single-threaded phases (e.g., before starting threads or within build scripts) where you can justify soundness.[6][2]
Citations:
- 1: https://youtrack.jetbrains.com/issue/RUST-14610/stdenvsetvar-will-be-unsafe-in-2024-edition-but-RustRover-also-considers-it-as-unsafe-in-the-2021-edition
- 2: https://doc.rust-lang.org/std/env/fn.set_var.html
- 3: https://news.ycombinator.com/item?id=42796058
- 4: https://codeandbitters.com/rust-2024-upgrade/
- 5: Audit usages of
std::env::*_varin the compiler rust-lang/rust#130672 - 6: https://doc.rust-lang.org/edition-guide/rust-2024/newly-unsafe-functions.html
- 7: https://users.rust-lang.org/t/unsafe-std-set-var-change/112704
- 8: https://news.ycombinator.com/item?id=43383993
- 9: https://users.rust-lang.org/t/unsafe-std-set-var-change/112704?page=2
- 10: https://users.rust-lang.org/t/unsafe-std-set-var-change/112704?page=3
MCP Context Analysis:
📊 Deepwiki:
- "the CliWorld struct is the central state container for Cucumber tests and that each scenario receives a fresh CliWorld instance (ensures isolation)." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "concrete recommendation to add an env field (example: pub env: std::collections::HashMap<String, String>) to CliWorld and to initialize it via Default or the #[world(init = ...)] attribute so step definitions can access injected environment data." (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
- "link to additional documentation for behavioural testing in the repo: docs/behavioural-testing-in-rust-with-cucumber.md" (https://deepwiki.com/search/inject-environment-into-cucumb_d5ba428a-f4e4-4fed-a4e0-79ec4db26c33)
🏁 Script executed:
#!/bin/bash
# Determine the Rust edition for the main crate
rg -n 'edition' Cargo.tomlLength of output: 45
Retain unsafe and revise the SAFETY comment
std::env::set_var is newly marked unsafe in the Rust 2024 edition. Keep the unsafe block and update the comment to justify safety in our single-threaded teardown:
Locations:
- tests/world.rs (around line 73)
- tests/runner_tests.rs (lines 27, 44, 65)
Example diff for tests/world.rs:
- if let Some(path) = self.original_path.take() {
- // SAFETY: Rust 2024 marks `set_var` as unsafe. Dropping `CliWorld`
- // reinstates the original `PATH`, ensuring scenarios cannot leak
- // environment changes into subsequent tests.
- unsafe { std::env::set_var("PATH", path) }
- }
+ if let Some(path) = self.original_path.take() {
+ // SAFETY: `set_var` is unsafe in Rust 2024 due to potential data races
+ // on global environment. This runs in single-threaded teardown,
+ // restoring PATH to avoid cross-scenario leakage.
+ unsafe { std::env::set_var("PATH", path) }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(path) = self.original_path.take() { | |
| // SAFETY: Rust 2024 marks `set_var` as unsafe. Dropping `CliWorld` | |
| // reinstates the original `PATH`, ensuring scenarios cannot leak | |
| // environment changes into subsequent tests. | |
| unsafe { std::env::set_var("PATH", path) } | |
| } | |
| if let Some(path) = self.original_path.take() { | |
| // SAFETY: `set_var` is unsafe in Rust 2024 due to potential data races | |
| // on the global environment. This runs in single-threaded teardown, | |
| // restoring PATH to avoid cross-scenario leakage. | |
| unsafe { std::env::set_var("PATH", path) } | |
| } |
🤖 Prompt for AI Agents
In tests/world.rs around lines 69 to 74 (and also update tests/runner_tests.rs
at lines 27, 44, 65), retain the unsafe block around std::env::set_var and
revise the SAFETY comments to justify why the unsafe call is sound: state that
teardown runs single-threaded in tests (no concurrent env access), we restore
the original PATH taken from self.original_path, and no other code observes a
partially-updated environment; update each comment to explicitly assert
single-threaded teardown and restoration semantics as the reason the unsafe
set_var is safe.
|
Superseded. |
1 participant
Summary
set_varand reset PATH on drop for test isolationTesting
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_689a355f02d483228f58a6d116c00e0e
Summary by Sourcery
Refactor Cucumber test harness to inject a mockable environment into the CliWorld, defaulting to a MockEnv implementation, and restore the original PATH on drop to ensure test isolation.
New Features:
Enhancements:
Tests: