diff --git a/changelog/+pin-rich-render-env-in-tests.housekeeping.md b/changelog/+pin-rich-render-env-in-tests.housekeeping.md new file mode 100644 index 00000000..9b0c1d61 --- /dev/null +++ b/changelog/+pin-rich-render-env-in-tests.housekeeping.md @@ -0,0 +1 @@ +Pinned Rich's colour and width for the test suite from `pytest_configure`, so CLI-output assertions render identically regardless of the developer's `FORCE_COLOR`/`COLUMNS` environment. diff --git a/tests/AGENTS.md b/tests/AGENTS.md index cce67364..597c1003 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -59,8 +59,37 @@ def test_cli_command(): - Use `httpx_mock` fixture for HTTP mocking - Clean up resources in integration tests +- Let `tests/conftest.py` own Rich's rendering environment (see below) instead of pinning + colour or width per test 🚫 **Never** - Add `@pytest.mark.asyncio` (globally enabled) - Make unit tests depend on external services +- Set `TERM=dumb` to disable colour — it pins Rich's width to 80 and ignores `COLUMNS`, which + truncates the wide tables the CLI-output fixtures record + +## CLI output and Rich + +Tests that assert on CLI text compare against output whose colour and width Rich decides. Both +are pinned centrally by `pytest_configure` in `tests/conftest.py`, which unsets `FORCE_COLOR` +and sets `NO_COLOR=1` and `COLUMNS=200` before any test module is imported. + +The timing matters. Rich snapshots `no_color` when a `Console` is constructed, and treats *any* +`FORCE_COLOR` value — the empty string included — as proof it is writing to a terminal. Many +`infrahub_sdk.ctl` modules build a module-level `Console()`, which runs during collection, so a +fixture or an env override passed to `CliRunner.invoke()` is already too late for those consoles. + +Practical consequences: + +- A plain `CliRunner()` is fine; it inherits the pinned environment. Pass `env=` only to widen + `COLUMNS` beyond 200 for a specific test. +- When a test builds its own `Console`, make it explicit — + `Console(file=StringIO(), width=1000, no_color=True, force_terminal=False)` — so it does not + depend on the ambient environment at all. Prefer wrapping that in a fixture that patches the + module-level console and yields it, as `schema_console` in `tests/unit/sdk/test_schema.py` + does, rather than repeating the `mock.patch` block per test. +- Tests that cover the test infrastructure itself, rather than any SDK behaviour, live in + `tests/unit/meta/`. +- Prefer fixing the environment over loosening an assertion, so exact-output tests keep their + value. diff --git a/tests/conftest.py b/tests/conftest.py index 9098d373..b94f0bc9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,31 @@ ENV_VARS_TO_CLEAN = ["INFRAHUB_ADDRESS", "INFRAHUB_TOKEN", "INFRAHUB_BRANCH", "INFRAHUB_USERNAME", "INFRAHUB_PASSWORD"] +# Rendering environment for every test in this suite. +# +# Rich snapshots ``no_color`` when a ``Console`` is constructed, and a ``Console`` reports +# itself as a terminal if ``FORCE_COLOR`` is set to *any* value, empty string included. Many +# ``infrahub_sdk.ctl`` modules build a module-level ``Console()``, and that happens while pytest +# imports the test modules -- before any fixture can run. So the environment has to be pinned +# from a hook that runs ahead of collection, which is what ``pytest_configure`` does. +# +# Without this, a developer whose shell exports ``FORCE_COLOR`` (or whose terminal is narrower +# than the CLI-output fixtures) gets ANSI escapes and truncated Rich tables in captured output, +# and every test that asserts on CLI text fails locally while staying green in CI. +# +# ``TERM`` is deliberately left alone: ``TERM=dumb`` puts Rich in its dumb-terminal path, which +# pins the width to 80 and ignores ``COLUMNS``, truncating the wide tables these fixtures record. +RENDER_ENV = {"NO_COLOR": "1", "COLUMNS": "200"} +RENDER_ENV_TO_CLEAN = ["FORCE_COLOR"] + + +def pytest_configure(config: pytest.Config) -> None: + """Pin Rich's colour and width before any test module is imported.""" + for name in RENDER_ENV_TO_CLEAN: + os.environ.pop(name, None) + os.environ.update(RENDER_ENV) + + def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: pytest_asyncio_tests = (item for item in items if pytest_asyncio.is_async_test(item)) session_scope_marker = pytest.mark.asyncio(loop_scope="session") diff --git a/tests/unit/meta/__init__.py b/tests/unit/meta/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/meta/test_render_env.py b/tests/unit/meta/test_render_env.py new file mode 100644 index 00000000..6809fcfb --- /dev/null +++ b/tests/unit/meta/test_render_env.py @@ -0,0 +1,67 @@ +"""Guards for the Rich rendering environment pinned in ``tests/conftest.py``. + +CLI-output assertions elsewhere in this suite compare against text whose colour and width Rich +decides. ``pytest_configure`` pins that decision; these tests pin the pinning, so a future edit +that loosens it fails here with an explanation rather than as a scatter of puzzling +output-comparison failures across the ctl tests. +""" + +from __future__ import annotations + +import os +from io import StringIO + +import pytest +from rich.console import Console + +from tests.conftest import RENDER_ENV, RENDER_ENV_TO_CLEAN + + +def test_render_env_is_pinned() -> None: + """The hook in conftest ran, and ran before this module was imported.""" + for name in RENDER_ENV_TO_CLEAN: + assert name not in os.environ, f"{name} must be unset: Rich reads any value as 'this is a terminal'" + for name, value in RENDER_ENV.items(): + assert os.environ.get(name) == value + + +@pytest.mark.parametrize("term", ["dumb", "unknown", "xterm-256color", "screen", ""]) +def test_console_width_survives_any_term(term: str, monkeypatch: pytest.MonkeyPatch) -> None: + """``TERM`` must not change the rendered width, whatever the developer's shell exports. + + Rich clamps the width to 80 and ignores ``COLUMNS`` on a *dumb* terminal, but + ``is_dumb_terminal`` is ``is_terminal and TERM in ("dumb", "unknown")`` -- so the clamp needs + Rich to also believe it is on a terminal. Captured test output never is, and the conftest hook + removes the one variable (``FORCE_COLOR``) that would make Rich claim otherwise. That is why + the hook does not pin ``TERM``, and why pinning it to a non-dumb value would be a no-op. + """ + monkeypatch.setenv("TERM", term) + + console = Console(file=StringIO()) + + assert console.is_terminal is False + assert console.is_dumb_terminal is False + assert console.width == int(RENDER_ENV["COLUMNS"]) + assert console.no_color is True + + +@pytest.mark.parametrize("force_color", ["1", "3", ""]) +def test_force_color_is_what_would_clamp_the_width(force_color: str, monkeypatch: pytest.MonkeyPatch) -> None: + """Pin the failure mode the hook exists to prevent. + + With ``FORCE_COLOR`` set, Rich treats the captured output as a terminal; combined with + ``TERM=dumb`` that drops it to width 80, which truncates the wide tables the CLI-output + fixtures record. This is the state a developer's shell puts the suite in, and the reason + ``FORCE_COLOR`` is removed rather than merely overridden. + + The empty string is the case that makes *removal* the only correct fix: Rich tests + ``FORCE_COLOR is not None``, so ``export FORCE_COLOR=`` forces a terminal just as ``1`` does, + and overriding the variable with a falsy value would not defuse it. + """ + monkeypatch.setenv("FORCE_COLOR", force_color) + monkeypatch.setenv("TERM", "dumb") + + console = Console(file=StringIO()) + + assert console.is_dumb_terminal is True + assert console.width == 80 diff --git a/tests/unit/sdk/test_schema.py b/tests/unit/sdk/test_schema.py index 2e134bce..25d41052 100644 --- a/tests/unit/sdk/test_schema.py +++ b/tests/unit/sdk/test_schema.py @@ -1,4 +1,5 @@ import inspect +from collections.abc import Generator from io import StringIO from unittest import mock from unittest.mock import MagicMock @@ -362,6 +363,22 @@ async def test_python_transform_config_description() -> None: assert config_explicit_none.description is None +@pytest.fixture +def schema_console() -> Generator[Console, None, None]: + """Capture what ``infrahub_sdk.ctl.schema`` prints, with colour and width pinned. + + ``width`` is set high so the error lines under test are never wrapped, and ``no_color`` / + ``force_terminal`` are explicit so the captured text does not depend on the ambient + environment even though ``tests/conftest.py`` already pins it. + + Yields: + Console: The console patched in place of ``infrahub_sdk.ctl.schema.console``. + """ + console = Console(file=StringIO(), width=1000, no_color=True, force_terminal=False) + with mock.patch("infrahub_sdk.ctl.schema.console", console): + yield console + + @mock.patch( "infrahub_sdk.ctl.schema.get_node", return_value={ @@ -370,7 +387,7 @@ async def test_python_transform_config_description() -> None: "attributes": [{"name": "name", "kind": "Text"}, {"name": "status", "kind": "Dropdown"}], }, ) -async def test_display_schema_load_errors_details_dropdown(mock_get_node: MagicMock) -> None: +async def test_display_schema_load_errors_details_dropdown(mock_get_node: MagicMock, schema_console: Console) -> None: """Validate error message with details when loading schema.""" error = { "detail": [ @@ -385,14 +402,14 @@ async def test_display_schema_load_errors_details_dropdown(mock_get_node: MagicM ] } - with mock.patch("infrahub_sdk.ctl.schema.console", Console(file=StringIO(), width=1000)) as console: - display_schema_load_errors(response=error, schemas_data=[]) - mock_get_node.assert_called_once() - output = console.file.getvalue() - expected_console = """Unable to load the schema: + display_schema_load_errors(response=error, schemas_data=[]) + + mock_get_node.assert_called_once() + output = schema_console.file.getvalue() + expected_console = """Unable to load the schema: Node: CloudInstance | Attribute: status ({'name': 'status', 'kind': 'Dropdown'}) | Value error, The property 'choices' is required for kind=Dropdown (value_error) """ # noqa: E501 - assert output == expected_console + assert output == expected_console @mock.patch( @@ -403,7 +420,7 @@ async def test_display_schema_load_errors_details_dropdown(mock_get_node: MagicM "attributes": [{"name": "name", "kind": "Text"}, {"name": "status", "kind": "Dropdown"}], }, ) -async def test_display_schema_load_errors_details_namespace(mock_get_node: MagicMock) -> None: +async def test_display_schema_load_errors_details_namespace(mock_get_node: MagicMock, schema_console: Console) -> None: """Validate error message with details when loading schema.""" error = { "detail": [ @@ -418,14 +435,14 @@ async def test_display_schema_load_errors_details_namespace(mock_get_node: Magic ] } - with mock.patch("infrahub_sdk.ctl.schema.console", Console(file=StringIO(), width=1000)) as console: - display_schema_load_errors(response=error, schemas_data=[]) - mock_get_node.assert_called_once() - output = console.file.getvalue() - expected_console = """Unable to load the schema: + display_schema_load_errors(response=error, schemas_data=[]) + + mock_get_node.assert_called_once() + output = schema_console.file.getvalue() + expected_console = """Unable to load the schema: Node: OuTInstance | namespace (OuT) | String should match pattern '^[A-Z][a-z0-9]+$' (string_pattern_mismatch) """ - assert output == expected_console + assert output == expected_console @mock.patch( @@ -459,7 +476,7 @@ async def test_display_schema_load_errors_details_namespace(mock_get_node: Magic }, ) async def test_display_schema_load_errors_details_when_error_is_in_attribute_or_relationship( - mock_get_node: MagicMock, + mock_get_node: MagicMock, schema_console: Console ) -> None: """Validate error message with details when loading schema and errors are in attribute or relationship.""" error = { @@ -479,15 +496,15 @@ async def test_display_schema_load_errors_details_when_error_is_in_attribute_or_ ] } - with mock.patch("infrahub_sdk.ctl.schema.console", Console(file=StringIO(), width=1000)) as console: - display_schema_load_errors(response=error, schemas_data=[]) - assert mock_get_node.call_count == 2 - output = console.file.getvalue() - expected_console = """Unable to load the schema: + display_schema_load_errors(response=error, schemas_data=[]) + + assert mock_get_node.call_count == 2 + output = schema_console.file.getvalue() + expected_console = """Unable to load the schema: Node: SecurityTailscaleSSHRule | Attribute: check_period (0) | Extra inputs are not permitted (extra_forbidden) Node: SecurityTailscaleSSHRule | Attribute: check_period (10080) | Extra inputs are not permitted (extra_forbidden) """ - assert output == expected_console + assert output == expected_console @pytest.mark.parametrize(