Add env vars that are excluded from the process cache key

docs/notes/2.34.x.md

@@ -33,6 +33,16 @@ Pants option config files are now parsed as TOML 1.1 rather than TOML 1.0. This
 
 ### Goals
 
+The `test` goal has a new `[test].uncached_env_vars` option, and test targets a matching
+`uncached_env_vars` field, for environment variables that should reach test processes without
+contributing to their cache key. This is for values that unavoidably vary between runs but cannot
+change a test's result — a CI build id used to label a report, a token for uploading results.
+Naming such a variable in `extra_env_vars` gives every run a distinct cache key, so no test result
+is ever reused.
+
+Note that on a cache hit the test does not run, so these values are not observed at all, and that
+they are dropped under remote execution.
+
 ### Backends
 
 #### Docker
@@ -93,6 +103,11 @@ Linked Go binaries and test binaries now record a Go toolchain build ID, as `go
 
 ### Plugin API changes
 
+`Process` has a new `uncached_env` field, holding environment variables that are excluded from the
+process's cache key and merged into its environment after the cache is consulted. Use it only for
+values that cannot change what the process produces; see the field's documentation for the
+caveats.
+
 `Target`, `TargetAdaptor`, `SourceBlock`, `SourceBlocks`, `TextBlock` and `Hunk` are now backed by native Rust implementations. They are still importable from their previous locations and their public constructors, attributes and methods are unchanged, but they are no longer Python dataclasses and they are built in `__new__` rather than `__init__`, so `dataclasses.is_dataclass`, `dataclasses.fields` and `dataclasses.replace` no longer apply to them. Defining subclasses in Python, and setting attributes on those subclasses, continues to work as before.
 
 `Field` subclasses now resolve `default`, `required` and `none_is_valid_value`, and whether `removal_version` is set, once per field type and cache the result. A field type that computes any of these dynamically, for example through a descriptor returning a different value on each access, will now only ever see the first value.

src/python/pants/backend/python/goals/pytest_runner.py

@@ -188,6 +188,7 @@ class TestMetadata:
 
     interpreter_constraints: InterpreterConstraints
     extra_env_vars: tuple[str, ...]
+    uncached_env_vars: tuple[str, ...]
     xdist_concurrency: int | None
     resolve: str
     environment: str
@@ -298,19 +299,25 @@ async def setup_pytest_for_target(
         EnvironmentVarsRequest(request.metadata.extra_env_vars), **implicitly()
     )
 
+    field_set_uncached_env_get = environment_vars_subset(
+        EnvironmentVarsRequest(request.metadata.uncached_env_vars), **implicitly()
+    )
+
     (
         pytest_pex,
         requirements_pex,
         prepared_sources,
         field_set_source_files,
         field_set_extra_env,
+        field_set_uncached_env,
         extra_output_directory_digest,
     ) = await concurrently(
         pytest_pex_get,
         requirements_pex_get,
         prepared_sources_get,
         field_set_source_files_get,
         field_set_extra_env_get,
+        field_set_uncached_env_get,
         extra_output_directory_digest_get,
     )
 
@@ -421,6 +428,12 @@ async def setup_pytest_for_target(
         **field_set_extra_env,
     }
 
+    uncached_env = {
+        **test_extra_env.uncached_env,
+        # Same precedence rule as `extra_env` above: the target's own value wins.
+        **field_set_uncached_env,
+    }
+
     # Cache test runs only if they are successful, or not at all if `--test-force`.
     cache_scope = test_subsystem.default_process_cache_scope
 
@@ -464,6 +477,7 @@ async def setup_pytest_for_target(
                 *field_set_source_files.files,
             ),
             extra_env=extra_env,
+            uncached_env=uncached_env,
             input_digest=input_digest,
             output_directories=(_EXTRA_OUTPUT_DIR,),
             output_files=output_files,
@@ -501,6 +515,7 @@ async def partition_python_tests(
                 [field_set], python_setup
             ),
             extra_env_vars=field_set.extra_env_vars.sorted(),
+            uncached_env_vars=field_set.uncached_env_vars.sorted(),
             xdist_concurrency=field_set.xdist_concurrency.value,
             resolve=field_set.resolve.normalized_value(python_setup),
             environment=field_set.environment.value,

src/python/pants/backend/python/goals/pytest_runner_integration_test.py

@@ -588,6 +588,45 @@ def test_args():
     assert result.exit_code == 0
 
 
+def test_uncached_env_vars(rule_runner: PythonRuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            f"{PACKAGE}/test_uncached_env_vars.py": dedent(
+                """\
+                import os
+
+                def test_args():
+                    assert os.getenv("ARG_UNCACHED_WITH_VALUE") == "arg_uncached_with_value"
+                    assert os.getenv("ARG_UNCACHED_WITHOUT_VALUE") == "arg_uncached_without_value"
+                    assert os.getenv("TARGET_UNCACHED_VAR") == "target_uncached_var"
+                    # A name in both lists resolves to the cached value: it is the more specific
+                    # request, and honouring it keeps the process's key honest.
+                    assert os.getenv("IN_BOTH_LISTS") == "from_extra_env_vars"
+                """
+            ),
+            f"{PACKAGE}/BUILD": dedent(
+                """\
+                python_tests(
+                    uncached_env_vars=("TARGET_UNCACHED_VAR=target_uncached_var",),
+                    extra_env_vars=("IN_BOTH_LISTS=from_extra_env_vars",),
+                )
+                """
+            ),
+        }
+    )
+    tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="test_uncached_env_vars.py"))
+    result = run_pytest(
+        rule_runner,
+        [tgt],
+        extra_args=[
+            "--test-uncached-env-vars=['ARG_UNCACHED_WITH_VALUE=arg_uncached_with_value', "
+            "'ARG_UNCACHED_WITHOUT_VALUE', 'IN_BOTH_LISTS=from_uncached_env_vars']"
+        ],
+        env={"ARG_UNCACHED_WITHOUT_VALUE": "arg_uncached_without_value"},
+    )
+    assert result.exit_code == 0
+
+
 def test_pytest_addopts_test_extra_env(rule_runner: PythonRuleRunner) -> None:
     rule_runner.write_files(
         {
@@ -875,6 +914,18 @@ def test_debug_adaptor_request_argv(rule_runner: PythonRuleRunner) -> None:
             "python_tests(overrides={'test_2.py': {'extra_env_vars': ['BAR', 'FOO']}})",
             [[f"{PACKAGE}/test_1.py", f"{PACKAGE}/test_2.py", f"{PACKAGE}/test_3.py"]],
         ],
+        # A batch runs as one process, so targets wanting different uncached env must split too:
+        [
+            "__defaults__(dict(python_tests=dict(batch_compatibility_tag='default', uncached_env_vars=['BUILD_ID'])))",
+            "python_tests(overrides={'test_2.py': {'uncached_env_vars': []}})",
+            [[f"{PACKAGE}/test_1.py", f"{PACKAGE}/test_3.py"], [f"{PACKAGE}/test_2.py"]],
+        ],
+        # Order of uncached_env_vars shouldn't affect partitioning:
+        [
+            "__defaults__(dict(python_tests=dict(batch_compatibility_tag='default', uncached_env_vars=['FOO', 'BAR'])))",
+            "python_tests(overrides={'test_2.py': {'uncached_env_vars': ['BAR', 'FOO']}})",
+            [[f"{PACKAGE}/test_1.py", f"{PACKAGE}/test_2.py", f"{PACKAGE}/test_3.py"]],
+        ],
         # Partition on different environments:
         [
             "__defaults__(dict(python_tests=dict(batch_compatibility_tag='default')))",

src/python/pants/backend/python/subsystems/pytest.py

@@ -16,6 +16,7 @@
     PythonTestsExtraEnvVarsField,
     PythonTestSourceField,
     PythonTestsTimeoutField,
+    PythonTestsUncachedEnvVarsField,
     PythonTestsXdistConcurrencyField,
     SkipPythonTestsField,
 )
@@ -41,6 +42,7 @@ class PythonTestFieldSet(TestFieldSet):
     timeout: PythonTestsTimeoutField
     runtime_package_dependencies: RuntimePackageDependenciesField
     extra_env_vars: PythonTestsExtraEnvVarsField
+    uncached_env_vars: PythonTestsUncachedEnvVarsField
     xdist_concurrency: PythonTestsXdistConcurrencyField
     batch_compatibility_tag: PythonTestsBatchCompatibilityTagField
     resolve: PythonResolveField

src/python/pants/backend/python/target_types.py

@@ -29,6 +29,7 @@
     TestExtraEnvVarsField,
     TestsBatchCompatibilityTagField,
     TestSubsystem,
+    TestUncachedEnvVarsField,
 )
 from pants.core.target_types import ResolveLikeField, ResolveLikeFieldToValueRequest
 from pants.engine.addresses import Address, Addresses
@@ -1357,6 +1358,10 @@ class PythonTestsExtraEnvVarsField(TestExtraEnvVarsField):
     pass
 
 
+class PythonTestsUncachedEnvVarsField(TestUncachedEnvVarsField):
+    pass
+
+
 class PythonTestsXdistConcurrencyField(IntField):
     alias = "xdist_concurrency"
     help = help_text(
@@ -1397,6 +1402,7 @@ class SkipPythonTestsField(BoolField):
     PythonTestsBatchCompatibilityTagField,
     RuntimePackageDependenciesField,
     PythonTestsExtraEnvVarsField,
+    PythonTestsUncachedEnvVarsField,
     InterpreterConstraintsField,
     SkipPythonTestsField,
     EnvironmentField,

src/python/pants/backend/python/util_rules/pex.py

@@ -1318,6 +1318,7 @@ class PexProcess:
     input_digest: Digest | None
     working_directory: str | None
     extra_env: FrozenDict[str, str]
+    uncached_env: FrozenDict[str, str]
     output_files: tuple[str, ...] | None
     output_directories: tuple[str, ...] | None
     timeout_seconds: int | None
@@ -1335,6 +1336,7 @@ def __init__(
         input_digest: Digest | None = None,
         working_directory: str | None = None,
         extra_env: Mapping[str, str] | None = None,
+        uncached_env: Mapping[str, str] | None = None,
         output_files: Iterable[str] | None = None,
         output_directories: Iterable[str] | None = None,
         timeout_seconds: int | None = None,
@@ -1349,6 +1351,7 @@ def __init__(
         object.__setattr__(self, "input_digest", input_digest)
         object.__setattr__(self, "working_directory", working_directory)
         object.__setattr__(self, "extra_env", FrozenDict(extra_env or {}))
+        object.__setattr__(self, "uncached_env", FrozenDict(uncached_env or {}))
         object.__setattr__(self, "output_files", tuple(output_files) if output_files else None)
         object.__setattr__(
             self, "output_directories", tuple(output_directories) if output_directories else None
@@ -1383,6 +1386,7 @@ async def setup_pex_process(request: PexProcess, pex_environment: PexEnvironment
         input_digest=input_digest,
         working_directory=request.working_directory,
         env=env,
+        uncached_env=request.uncached_env,
         output_files=request.output_files,
         output_directories=request.output_directories,
         append_only_caches={
@@ -1405,6 +1409,7 @@ class VenvPexProcess:
     input_digest: Digest | None
     working_directory: str | None
     extra_env: FrozenDict[str, str]
+    uncached_env: FrozenDict[str, str]
     output_files: tuple[str, ...] | None
     output_directories: tuple[str, ...] | None
     timeout_seconds: int | None
@@ -1423,6 +1428,7 @@ def __init__(
         input_digest: Digest | None = None,
         working_directory: str | None = None,
         extra_env: Mapping[str, str] | None = None,
+        uncached_env: Mapping[str, str] | None = None,
         output_files: Iterable[str] | None = None,
         output_directories: Iterable[str] | None = None,
         timeout_seconds: int | None = None,
@@ -1438,6 +1444,7 @@ def __init__(
         object.__setattr__(self, "input_digest", input_digest)
         object.__setattr__(self, "working_directory", working_directory)
         object.__setattr__(self, "extra_env", FrozenDict(extra_env or {}))
+        object.__setattr__(self, "uncached_env", FrozenDict(uncached_env or {}))
         object.__setattr__(self, "output_files", tuple(output_files) if output_files else None)
         object.__setattr__(
             self, "output_directories", tuple(output_directories) if output_directories else None
@@ -1479,6 +1486,7 @@ async def setup_venv_pex_process(
         input_digest=input_digest,
         working_directory=request.working_directory,
         env=request.extra_env,
+        uncached_env=request.uncached_env,
         output_files=request.output_files,
         output_directories=request.output_directories,
         append_only_caches=append_only_caches,

src/python/pants/core/goals/test.py

@@ -568,6 +568,30 @@ class EnvironmentAware:
                 """
             ),
         )
+        uncached_env_vars = StrListOption(
+            help=softwrap(
+                f"""
+                Additional environment variables to include in test processes, which do NOT
+                contribute to the test's cache key.
+
+                {EXTRA_ENV_VARS_USAGE_HELP}
+
+                Use this only for values that cannot change a test's result but unavoidably vary
+                between runs — a CI build id or job id used to label a report, a token for
+                uploading results. Such a value in `[test].extra_env_vars` gives every run a
+                distinct cache key, so no test result is ever reused.
+
+                Anything that can change whether a test passes belongs in `extra_env_vars`
+                instead. Putting it here means a cached result produced with a different value
+                gets reused.
+
+                Two limits are worth knowing. On a cache hit the test does not run, so these
+                values are not observed at all — they cannot be used to make something happen on
+                every run. And under remote execution they are dropped, because the only channel
+                to the worker is the request that the cache key is computed from.
+                """
+            ),
+        )
 
     debug = BoolOption(
         default=False,
@@ -818,6 +842,27 @@ def sorted(self) -> tuple[str, ...]:
         return tuple(sorted(self.value or ()))
 
 
+class TestUncachedEnvVarsField(StringSequenceField, metaclass=ABCMeta):
+    alias = "uncached_env_vars"
+    help = help_text(
+        f"""
+        Additional environment variables to include in test processes, which do NOT contribute to
+        the test's cache key.
+
+        {EXTRA_ENV_VARS_USAGE_HELP}
+
+        This will be merged with and override values from `[test].uncached_env_vars`.
+
+        See that option for when this is the right tool: values that unavoidably vary between runs
+        but cannot change whether a test passes. Anything that can change the result belongs in
+        `extra_env_vars`.
+        """
+    )
+
+    def sorted(self) -> tuple[str, ...]:
+        return tuple(sorted(self.value or ()))
+
+
 class TestsBatchCompatibilityTagField(StringField, metaclass=ABCMeta):
     alias = "batch_compatibility_tag"
 
@@ -1189,15 +1234,20 @@ def _format_test_rerun_command(results: Iterable[TestResult]) -> None | str:
 @dataclass(frozen=True)
 class TestExtraEnv:
     env: EnvironmentVars
+    uncached_env: EnvironmentVars
 
 
 @rule
 async def get_filtered_environment(test_env_aware: TestSubsystem.EnvironmentAware) -> TestExtraEnv:
-    return TestExtraEnv(
-        await environment_vars_subset(
+    env, uncached_env = await concurrently(
+        environment_vars_subset(
             EnvironmentVarsRequest(test_env_aware.extra_env_vars), **implicitly()
-        )
+        ),
+        environment_vars_subset(
+            EnvironmentVarsRequest(test_env_aware.uncached_env_vars), **implicitly()
+        ),
     )
+    return TestExtraEnv(env, uncached_env)
 
 
 @memoized

src/python/pants/engine/process.py

@@ -111,6 +111,7 @@ class Process:
     use_nailgun: tuple[str, ...]
     working_directory: str | None
     env: FrozenDict[str, str]
+    uncached_env: FrozenDict[str, str] = dataclasses.field(compare=False)
     append_only_caches: FrozenDict[str, str]
     output_files: tuple[str, ...]
     output_directories: tuple[str, ...]
@@ -134,6 +135,7 @@ def __init__(
         use_nailgun: Iterable[str] = (),
         working_directory: str | None = None,
         env: Mapping[str, str] | None = None,
+        uncached_env: Mapping[str, str] | None = None,
         append_only_caches: Mapping[str, str] | None = None,
         output_files: Iterable[str] | None = None,
         output_directories: Iterable[str] | None = None,
@@ -152,6 +154,15 @@ def __init__(
         that are not explicitly populated. For example, $PATH will not be defined by default, unless
         populated through the `env` parameter.
 
+        `env` values are part of the process's cache key, so a value that varies per run gives the
+        process a key nothing else can match. `uncached_env` is the escape hatch for values that
+        unavoidably vary but cannot change what the process produces — a CI build id used only to
+        label a report, an upload token. Those are excluded from the cache key and merged into the
+        environment after the cache is consulted. You own that invariant: if a value there can
+        change the output, cached results will be wrong. Note also that on a cache hit the process
+        does not run, so `uncached_env` cannot be used to make something happen on every run, and
+        that it is dropped under remote execution.
+
         Usually, you will want to provide input files/directories via the parameter `input_digest`.
         The process will then be able to access these paths through relative paths. If you want to
         give multiple input digests, first merge them with `merge_digests()`. Files larger than
@@ -187,6 +198,7 @@ def __init__(
         object.__setattr__(self, "use_nailgun", tuple(use_nailgun))
         object.__setattr__(self, "working_directory", working_directory)
         object.__setattr__(self, "env", FrozenDict(env or {}))
+        object.__setattr__(self, "uncached_env", FrozenDict(uncached_env or {}))
         object.__setattr__(self, "append_only_caches", FrozenDict(append_only_caches or {}))
         object.__setattr__(self, "output_files", tuple(output_files or ()))
         object.__setattr__(self, "output_directories", tuple(output_directories or ()))
@@ -483,7 +495,10 @@ def from_process(
     ) -> InteractiveProcess:
         return InteractiveProcess(
             argv=process.argv,
-            env=process.env,
+            # An interactive process is never cached, so the distinction `uncached_env` draws does
+            # not apply here and the values simply belong in the environment. Folding them in keeps
+            # `--debug` faithful to a normal run; leaving them out silently drops them.
+            env={**process.uncached_env, **process.env},
             description=process.description,
             input_digest=process.input_digest,
             forward_signals_to_process=forward_signals_to_process,