-
Notifications
You must be signed in to change notification settings - Fork 0
Inject an Env seam into the manifest env() Jinja function #484
Copy link
Copy link
Closed
Labels
concurrencyConcurrency, parallelism, and synchronization work, including races and deadlocks.Concurrency, parallelism, and synchronization work, including races and deadlocks.enhancementNew feature or requestNew feature or requestmediumRoadmap items to schedule within the current quarter. Clear scope, normal review cycles.Roadmap items to schedule within the current quarter. Clear scope, normal review cycles.refactorBehaviour-preserving restructuring that improves code health.Behaviour-preserving restructuring that improves code health.testingTest coverage, test infrastructure, and verification tooling work.Test coverage, test infrastructure, and verification tooling work.
Description
Metadata
Metadata
Assignees
Labels
concurrencyConcurrency, parallelism, and synchronization work, including races and deadlocks.Concurrency, parallelism, and synchronization work, including races and deadlocks.enhancementNew feature or requestNew feature or requestmediumRoadmap items to schedule within the current quarter. Clear scope, normal review cycles.Roadmap items to schedule within the current quarter. Clear scope, normal review cycles.refactorBehaviour-preserving restructuring that improves code health.Behaviour-preserving restructuring that improves code health.testingTest coverage, test infrastructure, and verification tooling work.Test coverage, test infrastructure, and verification tooling work.
Context
The manifest
env()Jinja function reads the process environment directly atsrc/manifest/mod.rs:85:The function has three distinct outcomes — present,
NotPresent, andNotUnicode— and the last two are precisely the cases that are awkward to reach without mutating the real environment. Under the AGENTS.md testing mandate, in-process mutation is no longer available, so these branches currently have no legitimate route to coverage.The doc comment on
env_varalready advertises atest_support::env::VarGuardthat mutates global state; that example must go with the implementation.Required work
env_varan injected environment, and carry it on the value registered into the Jinja environment somanifest::from_strcan supply one.process_env_reader) at the manifest public boundary.env_vardoc example to construct aMockEnvrather than a mutating guard.tests/manifest_env_tests.rsonto the injected seam, covering all three outcomes including the non-UTF-8 branch.Acceptance criteria
std::env::varcall remains insrc/manifest/mod.rs.tests/manifest_env_tests.rsno longer acquiresEnvLockor mutates the process environment.NotUnicodebranch is covered by a test.make check-fmt,make lint, andmake testpass.Amended: the seam is a closure (
EnvReader), not&impl mockable::Env.The requirement above has been rewritten to match; see the discussion below for
the reasoning.
mockableis a dev-dependency, and the proportionality argumentis that a production dependency is a large price for reading one variable when
the repository already uses closure seams of the same shape in
stdlib::path,output_mode,output_prefs, andtheme.