functional2: forbid chdir and setting environment

These are a footgun and are not acceptable in functional2 due to thread
safety, effects on other tests, etc.

Co-authored-by: Commentator2.0 <lix@crystal-cavern.systems>

Change-Id: I8d7285061eaa9bab27edd52f3646024c8cf605e5

doc/manual/src/contributing/testing.md

@@ -62,6 +62,12 @@ For `installcheck` specifically, first run `just install` before running the tes
 
 Finer-grained filtering within a test suite is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option to a test suite executable, or the `GTEST_FILTER` environment variable.
 
+### Inspecting failures
+
+The test suite emits logs in `build/meson-logs/`; the full textual failure logs are in `build/meson-logs/testlog.txt`.
+
+If you want a much nicer experience of viewing the logs in a structured manner, use `xunit-viewer --results build/meson-logs/testlog.junit.xml --server` to view them in a web browser.
+
 ### Unit test support libraries
 
 There are headers and code which are not just used to test the library in question, but also downstream libraries.

tests/functional2/commands/test_custom_sub_commands.py

@@ -1,12 +1,8 @@
 import stat
 import sys
-
 import pytest
-
 from pathlib import Path
-
 from textwrap import dedent
-
 from functional2.testlib.fixtures.env import ManagedEnv
 from functional2.testlib.fixtures.nix import Nix
 

tests/functional2/pyproject.toml

@@ -102,6 +102,8 @@ task-tags = ["TODO", "FIXME", "XXX"]
 #   SIM2: double negation in comparasions (`not a == b` instead of `a != b` etc)
 #   SIM3: yoda conditions use `foo == "Foo"` instead of `"Foo" == foo`
 #   SIM9: defaults for get and zip
+# TID: banning of certain imports
+#   TID251: ban certain APIs
 # TD: enforce TODO comment style
 #   disabled TD001: allow FIXME and XXX comments
 #   disabled TD003: todos don't require an explicit issue link for us
@@ -151,7 +153,6 @@ task-tags = ["TODO", "FIXME", "XXX"]
 # PYI: things for pyi files
 # SLF: accessing of private members
 # SLOT: subclassing of builtin-types (str, tuple, namedtuple) related issues
-# TID: banning of certain imports
 # TC: typechecking imports (typechecking imports required to be inside of a `if TYPECHECKING` block
 # FLY: string joins
 # I: import sorting; covered by other rulesets
@@ -162,7 +163,7 @@ task-tags = ["TODO", "FIXME", "XXX"]
 # PGH: things about pygrep, we don't use
 # FURB: covered by other rule sets
 # TRY: try and raise related things, not helpful as we only do testing
-select = ["E4", "E7", "E9", "F", "ERA", "ASYNC", "ANN0", "ANN2", "A", "C4", "EM", "ISC", "INP", "LOG", "G", "PIE", "T20", "PT", "Q", "RSE", "RET", "SIM", "TD", "ARG", "PTH", "N", "PERF", "PLC", "PLE", "UP", "RUF"]
+select = ["E4", "E7", "E9", "F", "ERA", "ASYNC", "ANN0", "ANN2", "A", "C4", "EM", "ISC", "INP", "LOG", "G", "PIE", "T20", "PT", "Q", "RSE", "RET", "SIM", "TID251", "TD", "ARG", "PTH", "N", "PERF", "PLC", "PLE", "UP", "RUF"]
 ignore = ["ANN002", "ANN003", "TD001", "TD003", "PLE1", "RUF005"]
 
 [tool.ruff.lint.per-file-ignores]
@@ -191,3 +192,35 @@ suppress-none-returning = true
 [tool.ruff.lint.flake8-errmsg]
 # Allow raw strings to be up to 10 characters in error messages
 max-string-length = 10
+
+[tool.ruff.lint.flake8-tidy-imports]
+
+[tool.ruff.lint.flake8-tidy-imports.banned-api]
+# Forbid symbols that are footguns in our codebase due to unsafe global state
+"os.environ".msg = """
+Environment variables are process wide and are unsafe to set as they interfere with other tests. Instead, use:
+  - env= in subprocess functions
+  - functional2.testlib.commands.Command.with_env for starting subprocesses
+  - functional2.testlib.environ for read-only access
+"""
+"os.environb".msg = """
+Environment variables are process wide and are unsafe to set as they interfere with other tests. Instead, use:
+  - env= in subprocess functions
+  - functional2.testlib.commands.Command.with_env for starting subprocesses
+  - functional2.testlib.environ for read-only access
+"""
+"os.putenv".msg = """
+Environment variables are process wide and are unsafe to set as they interfere with other tests. Instead, use:
+  - env= in subprocess functions
+  - functional2.testlib.commands.Command.with_env for starting subprocesses
+  - functional2.testlib.environ for read-only access
+"""
+"os.unsetenv".msg = """
+Environment variables are process wide and are unsafe to set as they interfere with other tests. Instead, use:
+  - env= in subprocess functions
+  - functional2.testlib.commands.Command.with_env for starting subprocesses
+  - functional2.testlib.environ for read-only access
+"""
+"os.chdir".msg = "Changing current directory is process wide and may interfere with other tests. Use cwd= while spawning a process instead."
+"os.fchdir".msg = "Changing current directory is process wide and may interfere with other tests. Use cwd= while spawning a process instead."
+"contextlib.chdir".msg = "Changing current directory is process wide and may interfere with other tests. Use cwd= while spawning a process instead."

tests/functional2/testlib/environ.py

@@ -0,0 +1,40 @@
+"""
+Safe (read-only) environment access. This exists entirely so that we can ban
+os.environ in our codebase.
+"""
+
+import collections.abc
+import os
+from typing import TypeVar, overload
+from collections.abc import Iterator
+
+_T = TypeVar("_T")
+
+
+class ReadonlyDict(collections.abc.Mapping[str, str]):
+    def __init__(self, inner: os._Environ[str]):
+        self.inner = inner
+
+    def __getitem__(self, key: str) -> str:
+        return self.inner[key]
+
+    def __iter__(self) -> Iterator[str]:
+        return iter(self.inner)
+
+    def __len__(self) -> int:
+        return len(self.inner)
+
+    @overload
+    def get(self, key: str, /) -> str | None: ...
+    @overload
+    def get(self, key: str, /, default: str | _T) -> str | _T: ...
+
+    def get(self, key: str, /, default: str | _T = None) -> str | _T:
+        return self.inner.get(key, default=default)
+
+    def copy(self) -> dict[str, str]:
+        return self.inner.copy()
+
+
+# SAFETY: wrapped to not allow mutation
+environ = ReadonlyDict(os.environ)  # noqa: TID251

tests/functional2/testlib/fixtures/command.py

@@ -11,7 +11,6 @@ import pytest
 from functional2.testlib.fixtures.env import ManagedEnv
 from functional2.testlib.terminal_code_eater import eat_terminal_codes
 
-
 logger = logging.getLogger(__name__)
 
 

tests/functional2/testlib/fixtures/env.py

@@ -1,10 +1,11 @@
 import dataclasses
 import logging
-import os
 import platform
 import shutil
 from pathlib import Path
 
+from functional2.testlib.environ import environ
+
 import pytest
 
 SLASHES_IN_STORE_PATH_UNTIL_PACKAGE = "/nix/store/hash-program_name/".count("/")
@@ -149,15 +150,15 @@ class _Dirs:
 class ManagedEnv:
     def __init__(self, tmp_path: Path):
         # Things fetched from the global env
-        build_shell = os.environ.get("BUILD_TEST_SHELL")
-        global_path = os.environ.get("PATH")
+        build_shell = environ.get("BUILD_TEST_SHELL")
+        global_path = environ.get("PATH")
         # `NIX_BIN_DIR` either propagated from us or set by meson
         # Set to the codebase internal output if started standalone
         # This is where the current lix binaries are located.
         # local import to avoid cyclic dependencies
         from functional2.testlib.utils import lix_base_folder  # noqa: PLC0415
 
-        lix_bin = Path(os.environ.get("NIX_BIN_DIR", lix_base_folder / "outputs/out/bin"))
+        lix_bin = Path(environ.get("NIX_BIN_DIR", lix_base_folder / "outputs/out/bin"))
 
         self._env = {}
         self.path = _ManagedPath(build_shell)

tests/functional2/testlib/fixtures/pytest_command.py

@@ -1,6 +1,5 @@
 import pytest
 from _pytest.fixtures import FixtureRequest
-
 from functional2.testlib.fixtures.command import Command
 from functional2.testlib.fixtures.env import ManagedEnv
 

tests/functional2/testlib/fixtures/snapshot.py

@@ -1,4 +1,3 @@
-import os
 from collections.abc import Callable
 from logging import Logger
 from pathlib import Path
@@ -9,6 +8,8 @@ import pytest
 from _pytest.config import Config
 from _pytest.fixtures import FixtureRequest
 
+from functional2.testlib.environ import environ
+
 
 def pytest_addoption(parser: pytest.Parser) -> None:
     """
@@ -26,9 +27,7 @@ def pytest_addoption(parser: pytest.Parser) -> None:
 
 @pytest.fixture(scope="session")
 def do_snapshot_update(request: FixtureRequest) -> bool:
-    return request.config.getoption("accept-tests") or (
-        os.environ.get("_NIX_TEST_ACCEPT") is not None
-    )
+    return request.config.getoption("accept-tests") or (environ.get("_NIX_TEST_ACCEPT") is not None)
 
 
 class Snapshot:

tests/functional2/testlib/fixtures/test_snapshot.py

@@ -25,6 +25,7 @@ def _get_f2_snapshot_files(additional_files: FileDeclaration) -> FileDeclaration
                 "functional2": {
                     "testlib": {
                         "__init__.py": CopyFile("../__init__.py"),
+                        "environ.py": CopyFile("../environ.py"),
                         "fixtures": {
                             "__init__.py": CopyFile("__init__.py"),
                             "snapshot.py": CopyFile("snapshot.py"),

tests/functional2/testlib/utils.py

@@ -1,11 +1,11 @@
 import builtins
-import os
 import types
 import typing
 from pathlib import Path
 from types import UnionType
 from typing import Any, Literal, get_args, get_origin
 
+from functional2.testlib.environ import environ
 from functional2.testlib.fixtures.file_helper import (
     CopyFile,
     CopyTree,
@@ -141,9 +141,9 @@ def get_global_asset(name: str) -> Fileish:
         return CopyTemplate(
             functional2_base_folder / "testlib" / "global_assets" / "config.nix.template",
             {
-                "system": os.environ.get("system"),  # noqa: SIM112 # system is actually lowercase here
+                "system": environ.get("system"),
                 # Either just the build shell or entire global path if we are darwin
-                "path": os.environ.get("BUILD_TEST_SHELL") or os.environ.get("PATH"),
+                "path": environ.get("BUILD_TEST_SHELL") or environ.get("PATH"),
             },
         )
     return CopyFile(functional2_base_folder / "testlib" / "global_assets" / name)