From 870ace1cfd424a9ed6e467632b5fae1f828cb856 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Tue, 8 Sep 2026 11:19:00 +0000 Subject: [PATCH 1/4] test: pin Rich colour and width so CLI-output tests stop failing locally Tests that assert on CLI text let Rich decide colour and width from the ambient environment, so a developer whose shell exports FORCE_COLOR got ANSI escapes and truncated Rich tables in captured output. 37 tests across test_marketplace_app.py, test_repository_app.py, test_task_app.py and test_schema.py failed locally while staying green in CI, which trains contributors to ignore red test output. Pin the rendering environment from pytest_configure in tests/conftest.py: unset FORCE_COLOR, set NO_COLOR=1 and COLUMNS=200. The hook has to run there rather than in a fixture: Rich snapshots no_color when a Console is constructed, and many infrahub_sdk.ctl modules build a module-level Console() during collection, before any fixture runs. Rich also treats any FORCE_COLOR value, empty string included, as proof it is writing to a terminal, so the variable has to be removed rather than blanked. TERM is deliberately left alone. TERM=dumb sends Rich down its dumb-terminal path, which pins the width to 80 and ignores COLUMNS, which would truncate the wide tables the CLI-output fixtures record. One central hook covers all 24 CliRunner call sites, so no per-test env plumbing is needed. The three consoles that tests/unit/sdk/test_schema.py builds itself are made explicit with no_color=True and force_terminal=False so they do not depend on the environment at all. The suite is now green under FORCE_COLOR=1 COLUMNS=40, NO_COLOR=1, TERM=dumb and a bare environment alike. Co-Authored-By: Claude Opus 5 --- ...n-rich-render-env-in-tests.housekeeping.md | 1 + tests/AGENTS.md | 25 +++++++++++++++++++ tests/conftest.py | 25 +++++++++++++++++++ tests/unit/sdk/test_schema.py | 12 ++++++--- 4 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 changelog/+pin-rich-render-env-in-tests.housekeeping.md 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 000000000..af6839647 --- /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` in `tests/conftest.py`, which unsets `FORCE_COLOR` and sets `NO_COLOR=1` and `COLUMNS=200` before any test module is imported. Previously a developer whose shell exported `FORCE_COLOR` got ANSI escapes and truncated Rich tables in captured CLI output, so 37 tests across `tests/unit/ctl/test_marketplace_app.py`, `test_repository_app.py`, `test_task_app.py` and `tests/unit/sdk/test_schema.py` failed locally while staying green in CI. Test-only change. diff --git a/tests/AGENTS.md b/tests/AGENTS.md index cce67364c..ba7414790 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -59,8 +59,33 @@ 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 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 9098d3732..b94f0bc94 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/sdk/test_schema.py b/tests/unit/sdk/test_schema.py index 2e134bce6..16b5beb97 100644 --- a/tests/unit/sdk/test_schema.py +++ b/tests/unit/sdk/test_schema.py @@ -385,7 +385,9 @@ 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: + with mock.patch( + "infrahub_sdk.ctl.schema.console", Console(file=StringIO(), width=1000, no_color=True, force_terminal=False) + ) as console: display_schema_load_errors(response=error, schemas_data=[]) mock_get_node.assert_called_once() output = console.file.getvalue() @@ -418,7 +420,9 @@ 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: + with mock.patch( + "infrahub_sdk.ctl.schema.console", Console(file=StringIO(), width=1000, no_color=True, force_terminal=False) + ) as console: display_schema_load_errors(response=error, schemas_data=[]) mock_get_node.assert_called_once() output = console.file.getvalue() @@ -479,7 +483,9 @@ 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: + with mock.patch( + "infrahub_sdk.ctl.schema.console", Console(file=StringIO(), width=1000, no_color=True, force_terminal=False) + ) as console: display_schema_load_errors(response=error, schemas_data=[]) assert mock_get_node.call_count == 2 output = console.file.getvalue() From 5c3756b3af8e3094053c2bc1bfbbdbaa5a6ede61 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Tue, 8 Sep 2026 12:04:14 +0000 Subject: [PATCH 2/4] test: pin the rendering-environment invariant with regression tests Review on #1332 asked whether TERM=dumb defeats the COLUMNS=200 pin, since pytest_configure leaves TERM alone. It does not, but nothing in the suite said so, so the question was fair. Rich clamps to width 80 and ignores COLUMNS only on a dumb terminal, and is_dumb_terminal is `is_terminal and TERM in ("dumb", "unknown")`. Captured test output is never a terminal, and the hook removes the one variable that would make Rich claim otherwise -- FORCE_COLOR, which Rich reads as proof of a terminal for any value. So removing FORCE_COLOR is what defuses the dumb path; pinning TERM would be a no-op. Add tests/unit/test_render_env.py to hold that: the hook's env is applied, the width survives TERM in dumb/unknown/xterm-256color/screen/empty, and the one combination that would clamp to 80 (FORCE_COLOR set with TERM=dumb) is pinned as the failure mode the hook exists to prevent. Co-Authored-By: Claude Opus 5 --- tests/unit/test_render_env.py | 62 +++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/unit/test_render_env.py diff --git a/tests/unit/test_render_env.py b/tests/unit/test_render_env.py new file mode 100644 index 000000000..b4a30d686 --- /dev/null +++ b/tests/unit/test_render_env.py @@ -0,0 +1,62 @@ +"""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 + + +def test_force_color_is_what_would_clamp_the_width(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. + """ + monkeypatch.setenv("FORCE_COLOR", "1") + monkeypatch.setenv("TERM", "dumb") + + console = Console(file=StringIO()) + + assert console.is_dumb_terminal is True + assert console.width == 80 From caada89dbb7644a4d8898cd2c03e284ebcb17a4b Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Tue, 8 Sep 2026 13:04:37 +0000 Subject: [PATCH 3/4] test: cover the empty FORCE_COLOR form in the render-env regression test The regression test pinned the FORCE_COLOR + TERM=dumb clamp with FORCE_COLOR=1 only, while the invariant it exists to document is that Rich tests `FORCE_COLOR is not None` -- so `export FORCE_COLOR=` forces a terminal exactly as `1` does. That empty form is the case that makes removing the variable the only correct fix rather than overriding it with a falsy value, and it was the one form the test never exercised. Parameterize over "1", "3" and "" ("3" being what a real shell exports). All three clamp to width 80, so the documented claim is now pinned instead of asserted in a comment. Raised by cubic-dev-ai on PR #1332. Co-Authored-By: Claude Opus 5 --- tests/unit/test_render_env.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_render_env.py b/tests/unit/test_render_env.py index b4a30d686..6809fcfb0 100644 --- a/tests/unit/test_render_env.py +++ b/tests/unit/test_render_env.py @@ -45,15 +45,20 @@ def test_console_width_survives_any_term(term: str, monkeypatch: pytest.MonkeyPa assert console.no_color is True -def test_force_color_is_what_would_clamp_the_width(monkeypatch: pytest.MonkeyPatch) -> None: +@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", "1") + monkeypatch.setenv("FORCE_COLOR", force_color) monkeypatch.setenv("TERM", "dumb") console = Console(file=StringIO()) From 8d901e899ded1586f798be202feb3b6f9cc8aa08 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 9 Sep 2026 15:05:56 +0000 Subject: [PATCH 4/4] =?UTF-8?q?test:=20address=20review=20=E2=80=94=20sche?= =?UTF-8?q?ma=5Fconsole=20fixture,=20meta=20package,=20shorter=20changelog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback from polmichel on #1332, all three non-blocking: Extract the repeated Console-patching block in test_schema.py into a schema_console fixture. The three display_schema_load_errors tests built the same captured console inline; they now request the fixture and read schema_console.file, which drops a level of indentation from each and leaves one place to change if the console shape moves again. Move test_render_env.py into a new tests/unit/meta package. It tests the test infrastructure rather than any SDK behaviour, so it does not belong next to the suites that cover the SDK itself. The package __init__ is empty, matching every other tests/unit subpackage. Trim the changelog fragment to one sentence. The detail it carried (which test modules failed, and why) is useful in the commit and on the card, not in the release notes. tests/AGENTS.md now points at the fixture as the preferred shape and records what tests/unit/meta is for. The reviewer also suggested moving test_packaging_metadata.py into the new package. That file is not on develop -- it lives on infrahub-develop (956c830, #1277) -- so there is nothing to move here; whoever lands it on develop can drop it straight into tests/unit/meta. Suite stays green at 1827 passed under a bare environment, FORCE_COLOR=1 COLUMNS=40, NO_COLOR=1, TERM=dumb, and FORCE_COLOR=3 with COLUMNS=40. Co-Authored-By: Claude Opus 5 --- ...n-rich-render-env-in-tests.housekeeping.md | 2 +- tests/AGENTS.md | 6 +- tests/unit/meta/__init__.py | 0 tests/unit/{ => meta}/test_render_env.py | 0 tests/unit/sdk/test_schema.py | 65 +++++++++++-------- 5 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 tests/unit/meta/__init__.py rename tests/unit/{ => meta}/test_render_env.py (100%) diff --git a/changelog/+pin-rich-render-env-in-tests.housekeeping.md b/changelog/+pin-rich-render-env-in-tests.housekeeping.md index af6839647..9b0c1d61d 100644 --- a/changelog/+pin-rich-render-env-in-tests.housekeeping.md +++ b/changelog/+pin-rich-render-env-in-tests.housekeeping.md @@ -1 +1 @@ -Pinned Rich's colour and width for the test suite from `pytest_configure` in `tests/conftest.py`, which unsets `FORCE_COLOR` and sets `NO_COLOR=1` and `COLUMNS=200` before any test module is imported. Previously a developer whose shell exported `FORCE_COLOR` got ANSI escapes and truncated Rich tables in captured CLI output, so 37 tests across `tests/unit/ctl/test_marketplace_app.py`, `test_repository_app.py`, `test_task_app.py` and `tests/unit/sdk/test_schema.py` failed locally while staying green in CI. Test-only change. +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 ba7414790..597c1003e 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -86,6 +86,10 @@ Practical consequences: `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. + 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/unit/meta/__init__.py b/tests/unit/meta/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/test_render_env.py b/tests/unit/meta/test_render_env.py similarity index 100% rename from tests/unit/test_render_env.py rename to tests/unit/meta/test_render_env.py diff --git a/tests/unit/sdk/test_schema.py b/tests/unit/sdk/test_schema.py index 16b5beb97..25d410522 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,16 +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, no_color=True, force_terminal=False) - ) 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( @@ -405,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": [ @@ -420,16 +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, no_color=True, force_terminal=False) - ) 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( @@ -463,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 = { @@ -483,17 +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, no_color=True, force_terminal=False) - ) 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(