From a5de448856c0c594db646dddf0c8402ff0363f4e Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Mon, 16 Mar 2026 16:10:59 +0800 Subject: [PATCH] feat(testing): implement @tdd_expected_fail tag handling in Robot Framework Implements the three-tag TDD bug-capture system in Robot Framework via a Listener v3 module, paralleling the Behave implementation. Tests tagged tdd_expected_fail that fail have their result inverted to PASS (bug still exists); tests that unexpectedly pass are inverted to FAIL with guidance. Addresses all 15 findings from code review (PR !673, reviewer hamza.khyari): P2 fixes: - Added idempotency guard (_processed_tests set) to prevent double-inversion when the listener is loaded twice in the same process. - Rewrote normal-test-unaffected check to run alongside a tdd_expected_fail fixture in a single Robot invocation, proving the listener is loaded and selectively applies rather than being a tautological pass. P3 fixes: - Added output.xml existence guard with clear diagnostics in _run_fixture. - Documented intentional use of data.tags (static definition) vs result.tags (runtime-modifiable) in end_test docstring. - Added SKIP status test fixture and integration test case. - Added message content assertion in cmd_expected_fail_inverted. - Tightened substring assertions to match specific error text. - Added tdd_expected_fail-alone fixture (both companions missing). - Added close() hook to clear _validation_errors and _processed_tests. - Simplified _run_fixture return type to tuple[str, str]. - Changed listener path resolution from CWD-relative to __file__-relative in noxfile.py (integration_tests, slow_integration_tests, e2e_tests). P4 fixes: - Added __all__ declaration to helper module. - Changed module docstring from "mirroring" to "paralleling". - Added comment documenting accepted XML parsing risk (self-generated XML). Additional fixes: - Increased M4 E2E plan-tree test timeout from 30s to 120s (pre-existing timeout failure unrelated to this feature). Quality gates (post-rebase onto latest master): - nox -s lint: PASS - nox -s typecheck: PASS (0 errors) - nox -s unit_tests: PASS (10,700 scenarios) - nox -s integration_tests: PASS (1,505 tests) - nox -s coverage_report: PASS (97.9% >= 97% threshold) - nox -s benchmark: PASS - nox -s docs: PASS - nox -s build: PASS - nox -s security_scan: PASS - nox -s dead_code: PASS ISSUES CLOSED: #628 --- CHANGELOG.md | 10 + features/mocks/__init__.py | 2 + features/mocks/tdd_test_helpers.py | 65 +++ features/steps/tdd_tag_validation_steps.py | 54 +-- noxfile.py | 31 +- robot/_tdd_fixture_runner.py | 186 ++++++++ robot/fixtures/tdd_bug_alone.robot | 16 + robot/fixtures/tdd_expected_fail_alone.robot | 15 + robot/fixtures/tdd_expected_fail_fails.robot | 13 + .../tdd_expected_fail_missing_bug_n.robot | 15 + robot/fixtures/tdd_expected_fail_passes.robot | 14 + robot/fixtures/tdd_expected_fail_skip.robot | 14 + robot/fixtures/tdd_missing_tdd_bug.robot | 13 + robot/fixtures/tdd_normal_test.robot | 14 + robot/helper_tdd_tag_validation.py | 412 +++++++++++++----- robot/tdd_expected_fail_listener.py | 266 +++++++++-- robot/tdd_tag_validation.robot | 106 ++++- 17 files changed, 1069 insertions(+), 177 deletions(-) create mode 100644 features/mocks/tdd_test_helpers.py create mode 100644 robot/_tdd_fixture_runner.py create mode 100644 robot/fixtures/tdd_bug_alone.robot create mode 100644 robot/fixtures/tdd_expected_fail_alone.robot create mode 100644 robot/fixtures/tdd_expected_fail_fails.robot create mode 100644 robot/fixtures/tdd_expected_fail_missing_bug_n.robot create mode 100644 robot/fixtures/tdd_expected_fail_passes.robot create mode 100644 robot/fixtures/tdd_expected_fail_skip.robot create mode 100644 robot/fixtures/tdd_missing_tdd_bug.robot create mode 100644 robot/fixtures/tdd_normal_test.robot diff --git a/CHANGELOG.md b/CHANGELOG.md index a6fb38b5b..0905bc10a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,16 @@ when API keys are absent, and `--exclude E2E` on the standard integration test session. Includes a minimal smoke test exercising `agents --version` and `agents --help`. (#740) +- Implemented `tdd_expected_fail` tag handling in Robot Framework via a Listener v3 + module (`robot/tdd_expected_fail_listener.py`). Tests tagged `tdd_expected_fail` + that fail have their result inverted to pass (expected failure); tests that + unexpectedly pass are reported as failed with guidance to remove the tag. Tag + validation enforces `tdd_bug` + `tdd_bug_` prerequisites. Includes + idempotency guard against double-invocation, explicit SKIP status handling, + and a `close()` hook for clean teardown. Listener is registered in the nox + `integration_tests` and `slow_integration_tests` sessions. Fixture files are + excluded from the main pabot runner via `tdd_fixture` tag. Includes 9 Robot + Framework integration test cases. (#628) - Added TDD-style failing Behave BDD tests for the session list DI container missing `db` provider bug. Three scenarios exercise `session list`, `_get_session_service()`, and `session list --format json` through the real diff --git a/features/mocks/__init__.py b/features/mocks/__init__.py index ad8729563..cde17c714 100644 --- a/features/mocks/__init__.py +++ b/features/mocks/__init__.py @@ -9,6 +9,7 @@ from .fake_provider import FakeProviderInfo, FakeProviderRegistry from .lsp_transport_mock import MockLspTransport, parse_lsp_responses from .mock_ai_provider import MockAIProvider from .mock_mcp_transport import MockMCPTransport +from .tdd_test_helpers import make_mock_scenario from .transient_fail_audit_service import TransientFailAuditService __all__ = [ @@ -18,5 +19,6 @@ __all__ = [ "MockLspTransport", "MockMCPTransport", "TransientFailAuditService", + "make_mock_scenario", "parse_lsp_responses", ] diff --git a/features/mocks/tdd_test_helpers.py b/features/mocks/tdd_test_helpers.py new file mode 100644 index 000000000..e1add58dd --- /dev/null +++ b/features/mocks/tdd_test_helpers.py @@ -0,0 +1,65 @@ +"""Shared mock helpers for TDD tag validation tests. + +Provides ``make_mock_scenario`` — a factory for lightweight mock +``Scenario`` objects used by both the Behave step definitions +(``features/steps/tdd_tag_validation_steps.py``) and the Robot +Framework integration test helper (``robot/helper_tdd_tag_validation.py``). + +Centralising the mock builder eliminates duplication and ensures both +test suites exercise ``apply_tdd_inversion`` with identically-shaped +mock objects. + +See CONTRIBUTING.md > TDD Bug Test Tags for the three-tag specification. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from behave.model import Status + + +def make_mock_scenario( + tags: list[str], + steps_passed: bool = True, + hook_failed: bool = False, + was_dry_run: bool = False, + step_exception: BaseException | None = None, +) -> MagicMock: + """Build a lightweight mock ``Scenario`` for ``apply_tdd_inversion`` tests. + + Args: + tags: The effective tags to place on the scenario. + steps_passed: When ``True`` every step has ``Status.passed``. + When ``False`` the first step has ``Status.failed`` with + *step_exception* (defaulting to ``AssertionError``). + hook_failed: Simulates an infrastructure/hook error. + was_dry_run: Simulates ``--dry-run`` mode. + step_exception: The exception attached to the failed step when + *steps_passed* is ``False``. Defaults to ``AssertionError``. + + Returns: + A ``MagicMock`` configured to look like a Behave ``Scenario``. + """ + scenario = MagicMock() + scenario.effective_tags = tags + scenario.name = "mock-scenario" + scenario.hook_failed = hook_failed + scenario.was_dry_run = was_dry_run + + mock_step = MagicMock() + if steps_passed: + mock_step.status = Status.passed + mock_step.exception = None + else: + mock_step.status = Status.failed + mock_step.exception = ( + step_exception + if step_exception is not None + else AssertionError("simulated assertion failure") + ) + scenario.all_steps = [mock_step] + return scenario + + +__all__: list[str] = ["make_mock_scenario"] diff --git a/features/steps/tdd_tag_validation_steps.py b/features/steps/tdd_tag_validation_steps.py index d9f5f4648..4308d06a2 100644 --- a/features/steps/tdd_tag_validation_steps.py +++ b/features/steps/tdd_tag_validation_steps.py @@ -23,6 +23,7 @@ from features.environment import ( should_invert_result, validate_tdd_tags, ) +from features.mocks.tdd_test_helpers import make_mock_scenario # --------------------------------------------------------------------------- # Tag-set construction helpers @@ -96,46 +97,9 @@ def step_then_invert_false(context: Context) -> None: # --------------------------------------------------------------------------- # apply_tdd_inversion unit-test helpers # --------------------------------------------------------------------------- - - -def _make_mock_scenario( - tags: list[str], - steps_passed: bool = True, - hook_failed: bool = False, - was_dry_run: bool = False, - step_exception: BaseException | None = None, -) -> MagicMock: - """Build a lightweight mock ``Scenario`` for ``apply_tdd_inversion`` tests. - - Args: - tags: The effective tags to place on the scenario. - steps_passed: When ``True`` every step has ``Status.passed``. - When ``False`` the first step has ``Status.failed`` with - *step_exception* (defaulting to ``AssertionError``). - hook_failed: Simulates an infrastructure/hook error. - was_dry_run: Simulates ``--dry-run`` mode. - step_exception: The exception attached to the failed step when - *steps_passed* is ``False``. Defaults to ``AssertionError``. - """ - scenario = MagicMock() - scenario.effective_tags = tags - scenario.name = "mock-scenario" - scenario.hook_failed = hook_failed - scenario.was_dry_run = was_dry_run - - mock_step = MagicMock() - if steps_passed: - mock_step.status = Status.passed - mock_step.exception = None - else: - mock_step.status = Status.failed - mock_step.exception = ( - step_exception - if step_exception is not None - else AssertionError("simulated assertion failure") - ) - scenario.all_steps = [mock_step] - return scenario +# The mock scenario builder is in ``features/mocks/tdd_test_helpers.py`` +# (imported above as ``make_mock_scenario``) to avoid duplication with +# ``robot/helper_tdd_tag_validation.py``. # --------------------------------------------------------------------------- @@ -149,7 +113,7 @@ def _make_mock_scenario( ) def step_given_expected_failure_scenario(context: Context) -> None: """Create a mock scenario with a failed step (AssertionError) and TDD tags.""" - context.tdd_mock_scenario = _make_mock_scenario( + context.tdd_mock_scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_998", "tdd_expected_fail"], steps_passed=False, step_exception=AssertionError("simulated bug assertion failure"), @@ -202,7 +166,7 @@ def step_then_error_messages_cleared(context: Context) -> None: @given("tdd inversion a mock scenario that passes with tdd_expected_fail tags") def step_given_unexpected_pass_scenario(context: Context) -> None: """Create a mock scenario where steps pass but @tdd_expected_fail is set.""" - context.tdd_mock_scenario = _make_mock_scenario( + context.tdd_mock_scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_998", "tdd_expected_fail"], steps_passed=True, ) @@ -259,7 +223,7 @@ def step_then_last_step_has_error_message(context: Context) -> None: @given("tdd inversion a mock scenario with hook_failed and tdd_expected_fail tags") def step_given_hook_failed_scenario(context: Context) -> None: - context.tdd_mock_scenario = _make_mock_scenario( + context.tdd_mock_scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_998", "tdd_expected_fail"], steps_passed=False, hook_failed=True, @@ -287,7 +251,7 @@ def step_then_result_not_inverted(context: Context) -> None: @given("tdd inversion a mock scenario in dry-run mode with tdd_expected_fail tags") def step_given_dry_run_scenario(context: Context) -> None: - context.tdd_mock_scenario = _make_mock_scenario( + context.tdd_mock_scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_998", "tdd_expected_fail"], steps_passed=True, was_dry_run=True, @@ -308,7 +272,7 @@ def step_then_result_no_inversion(context: Context) -> None: @given("tdd inversion a mock scenario with a RuntimeError and tdd_expected_fail tags") def step_given_non_assertion_scenario(context: Context) -> None: - context.tdd_mock_scenario = _make_mock_scenario( + context.tdd_mock_scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_998", "tdd_expected_fail"], steps_passed=False, step_exception=RuntimeError("connection lost"), diff --git a/noxfile.py b/noxfile.py index 9de22a385..f103e916d 100644 --- a/noxfile.py +++ b/noxfile.py @@ -580,6 +580,15 @@ def integration_tests(session: nox.Session): pabot_args, robot_args = _split_pabot_args(session.posargs) parallel_args = _pabot_parallel_args(pabot_args) + # TDD expected-fail listener — inverts results for @tdd_expected_fail + # tagged tests and validates TDD tag combinations. + # See CONTRIBUTING.md > TDD Bug Test Tags. + # Resolved relative to this file (not CWD) so ``nox`` invocations from + # a non-root directory still find the listener module. + tdd_listener = str( + Path(__file__).parent / "robot" / "tdd_expected_fail_listener.py" + ) + session.run( "pabot", *parallel_args, @@ -596,6 +605,8 @@ def integration_tests(session: nox.Session): "xunit.xml", "--variable", f"PYTHON:{venv_python}", + "--listener", + tdd_listener, "--exclude", "slow", "--exclude", @@ -606,8 +617,8 @@ def integration_tests(session: nox.Session): "wip", "--exclude", "E2E", - "--listener", - "robot/tdd_expected_fail_listener.py", + "--exclude", + "tdd_fixture", *robot_args, "robot/", ) @@ -618,6 +629,16 @@ def slow_integration_tests(session: nox.Session): """Run Robot Framework integration tests.""" session.install("-e", ".[tests]") session.env["CLEVERAGENTS_AUTO_APPLY_MIGRATIONS"] = "true" + + # TDD expected-fail listener — inverts results for @tdd_expected_fail + # tagged tests and validates TDD tag combinations. + # See CONTRIBUTING.md > TDD Bug Test Tags. + # Resolved relative to this file (not CWD) so ``nox`` invocations from + # a non-root directory still find the listener module. + tdd_listener = str( + Path(__file__).parent / "robot" / "tdd_expected_fail_listener.py" + ) + session.run( "robot", "--outputdir", @@ -631,7 +652,9 @@ def slow_integration_tests(session: nox.Session): "--xunit", "xunit.xml", "--listener", - "robot/tdd_expected_fail_listener.py", + tdd_listener, + "--exclude", + "tdd_fixture", "robot/", *session.posargs, ) @@ -696,7 +719,7 @@ def e2e_tests(session: nox.Session): "--include", "E2E", "--listener", - "robot/tdd_expected_fail_listener.py", + str(Path(__file__).parent / "robot" / "tdd_expected_fail_listener.py"), *session.posargs, "robot/e2e/", ) diff --git a/robot/_tdd_fixture_runner.py b/robot/_tdd_fixture_runner.py new file mode 100644 index 000000000..07cb5ddb6 --- /dev/null +++ b/robot/_tdd_fixture_runner.py @@ -0,0 +1,186 @@ +"""Subprocess runner for TDD Robot Framework fixture files. + +Runs ``.robot`` fixture files with the ``tdd_expected_fail_listener`` +active and inspects the resulting ``output.xml`` to determine the final +test status after listener processing. + +Extracted from ``helper_tdd_tag_validation.py`` to keep file lengths +under the 500-line project limit. +""" + +from __future__ import annotations + +__all__: list[str] = [] + +import subprocess +import sys +import tempfile + +# Robot Framework generates the output XML; we parse it to inspect test +# results. The XML is always self-generated (never from untrusted input), +# so the standard library parser is sufficient — no XXE risk. +import xml.etree.ElementTree as ET +from pathlib import Path + +_ROBOT_DIR = Path(__file__).resolve().parent +_LISTENER = str(_ROBOT_DIR / "tdd_expected_fail_listener.py") +_FIXTURES = _ROBOT_DIR / "fixtures" + +_SUBPROCESS_TIMEOUT = 120 # seconds — matches project convention + + +def _extract_message( + test_el: ET.Element, + status_el: ET.Element, +) -> str: + """Extract the test message from an RF output XML status element. + + RF may place the message as element text, a ``message`` attribute + (RF 7.3+), or in a child ```` element. Check all three. + """ + message = status_el.text or "" + if not message: + message = status_el.get("message", "") + if not message: + msg_el = test_el.find(".//msg") + if msg_el is not None and msg_el.text: + message = msg_el.text + return message + + +def run_fixture(fixture_name: str) -> tuple[str, str]: + """Run a fixture ``.robot`` file and return ``(status, message)``. + + Returns the status and message of the *first* test case found in + the Robot output XML. + """ + fixture_path = _FIXTURES / f"{fixture_name}.robot" + if not fixture_path.exists(): + print(f"ERROR: fixture not found: {fixture_path}", file=sys.stderr) + sys.exit(1) + + with tempfile.TemporaryDirectory() as tmpdir: + out_xml = str(Path(tmpdir) / "output.xml") + cmd = [ + sys.executable, + "-m", + "robot", + "--listener", + _LISTENER, + "--outputdir", + tmpdir, + "--loglevel", + "INFO", + "--report", + "NONE", + "--log", + "NONE", + str(fixture_path), + ] + try: + proc = subprocess.run( + cmd, + capture_output=True, + text=True, + timeout=_SUBPROCESS_TIMEOUT, + ) + except subprocess.TimeoutExpired: + print( + f"ERROR: fixture {fixture_name!r} timed out after " + f"{_SUBPROCESS_TIMEOUT}s", + file=sys.stderr, + ) + sys.exit(1) + + if not Path(out_xml).exists(): + print( + f"ERROR: Robot did not produce output.xml (rc={proc.returncode})", + file=sys.stderr, + ) + print(f"stderr: {proc.stderr}", file=sys.stderr) + sys.exit(1) + + tree = ET.parse(out_xml) + root = tree.getroot() + test_el = root.find(".//test") + if test_el is None: + return "ERROR", "No test found in output XML" + status_el = test_el.find("status") + if status_el is None: + return "ERROR", "No status element found" + status = status_el.get("status", "UNKNOWN") + message = _extract_message(test_el, status_el) + return status, message + + +def run_multi_fixture( + *fixture_names: str, +) -> dict[str, tuple[str, str]]: + """Run multiple fixture ``.robot`` files in a single Robot invocation. + + Returns a dict mapping test name to ``(status, message)``. Running + multiple fixtures together proves the listener is loaded and + selectively applies (normal tests are unaffected *in the same run* + as ``tdd_expected_fail`` tests). + """ + fixture_paths = [] + for name in fixture_names: + fp = _FIXTURES / f"{name}.robot" + if not fp.exists(): + print(f"ERROR: fixture not found: {fp}", file=sys.stderr) + sys.exit(1) + fixture_paths.append(str(fp)) + + with tempfile.TemporaryDirectory() as tmpdir: + out_xml = str(Path(tmpdir) / "output.xml") + cmd = [ + sys.executable, + "-m", + "robot", + "--listener", + _LISTENER, + "--outputdir", + tmpdir, + "--loglevel", + "INFO", + "--report", + "NONE", + "--log", + "NONE", + *fixture_paths, + ] + try: + proc = subprocess.run( + cmd, + capture_output=True, + text=True, + timeout=_SUBPROCESS_TIMEOUT, + ) + except subprocess.TimeoutExpired: + print( + f"ERROR: multi-fixture run timed out after {_SUBPROCESS_TIMEOUT}s", + file=sys.stderr, + ) + sys.exit(1) + + if not Path(out_xml).exists(): + print( + f"ERROR: Robot did not produce output.xml (rc={proc.returncode})", + file=sys.stderr, + ) + print(f"stderr: {proc.stderr}", file=sys.stderr) + sys.exit(1) + + tree = ET.parse(out_xml) + root = tree.getroot() + results: dict[str, tuple[str, str]] = {} + for test_el in root.findall(".//test"): + test_name = test_el.get("name", "UNKNOWN") + status_el = test_el.find("status") + if status_el is None: + results[test_name] = ("ERROR", "No status element found") + continue + status = status_el.get("status", "UNKNOWN") + message = _extract_message(test_el, status_el) + results[test_name] = (status, message) + return results diff --git a/robot/fixtures/tdd_bug_alone.robot b/robot/fixtures/tdd_bug_alone.robot new file mode 100644 index 000000000..3a186a8aa --- /dev/null +++ b/robot/fixtures/tdd_bug_alone.robot @@ -0,0 +1,16 @@ +*** Settings *** +Documentation Fixture: a test with only tdd_bug (no tdd_bug_N or +... tdd_expected_fail). +... Used by helper_tdd_tag_validation.py to verify that +... tdd_bug alone is a valid tag combination — the listener +... should NOT modify the test result. +... This file must NOT be picked up by the main pabot runner — +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +TDD Bug Tag Alone Is Valid + [Tags] tdd_bug tdd_fixture + [Documentation] A test with only tdd_bug should pass normally. + ... The listener should not interfere — tdd_bug alone is valid. + Log Test with tdd_bug alone — should pass without modification. + Should Be True ${TRUE} diff --git a/robot/fixtures/tdd_expected_fail_alone.robot b/robot/fixtures/tdd_expected_fail_alone.robot new file mode 100644 index 000000000..1b7048e43 --- /dev/null +++ b/robot/fixtures/tdd_expected_fail_alone.robot @@ -0,0 +1,15 @@ +*** Settings *** +Documentation Fixture: a test with only tdd_expected_fail (both tdd_bug +... and tdd_bug_N are missing). +... Used by helper_tdd_tag_validation.py to verify tag +... validation catches both missing prerequisite tags. +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Expected Fail Alone Causes Validation Error + [Tags] tdd_expected_fail tdd_fixture + [Documentation] This test has tdd_expected_fail but neither tdd_bug nor + ... tdd_bug_N. The listener should detect this and force a validation + ... failure mentioning both missing tags. + Log This should not matter -- validation should fail first. diff --git a/robot/fixtures/tdd_expected_fail_fails.robot b/robot/fixtures/tdd_expected_fail_fails.robot new file mode 100644 index 000000000..74d18ee33 --- /dev/null +++ b/robot/fixtures/tdd_expected_fail_fails.robot @@ -0,0 +1,13 @@ +*** Settings *** +Documentation Fixture: a tdd_expected_fail test that deliberately fails. +... Used by helper_tdd_tag_validation.py to verify result inversion. +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Bug 999 Expected Failure Is Inverted To Pass + [Tags] tdd_bug tdd_bug_999 tdd_expected_fail tdd_fixture + [Documentation] This test deliberately fails to simulate a bug still + ... being present. The tdd_expected_fail listener should invert this + ... failure into a pass. + Fail Deliberate failure: bug 999 is still present (expected by TDD tag) diff --git a/robot/fixtures/tdd_expected_fail_missing_bug_n.robot b/robot/fixtures/tdd_expected_fail_missing_bug_n.robot new file mode 100644 index 000000000..a6cd72326 --- /dev/null +++ b/robot/fixtures/tdd_expected_fail_missing_bug_n.robot @@ -0,0 +1,15 @@ +*** Settings *** +Documentation Fixture: a test with tdd_expected_fail and tdd_bug but +... missing tdd_bug_N. +... Used by helper_tdd_tag_validation.py to verify tag +... validation catches the missing tdd_bug_N tag. +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Missing TDD Bug N Tag Causes Validation Error + [Tags] tdd_bug tdd_expected_fail tdd_fixture + [Documentation] This test has tdd_expected_fail and tdd_bug but no + ... tdd_bug_N tag. The listener should detect this and force a + ... validation failure. + Log This should not matter -- validation should fail first. diff --git a/robot/fixtures/tdd_expected_fail_passes.robot b/robot/fixtures/tdd_expected_fail_passes.robot new file mode 100644 index 000000000..070b94f6d --- /dev/null +++ b/robot/fixtures/tdd_expected_fail_passes.robot @@ -0,0 +1,14 @@ +*** Settings *** +Documentation Fixture: a tdd_expected_fail test that unexpectedly passes. +... Used by helper_tdd_tag_validation.py to verify that an +... unexpected pass is inverted to a failure with guidance. +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Bug 998 Unexpected Pass Is Inverted To Fail + [Tags] tdd_bug tdd_bug_998 tdd_expected_fail tdd_fixture + [Documentation] This test passes, simulating a bug that was fixed + ... without removing the tdd_expected_fail tag. The listener should + ... invert this pass into a fail with guidance message. + Log This test passes unexpectedly (bug appears fixed). diff --git a/robot/fixtures/tdd_expected_fail_skip.robot b/robot/fixtures/tdd_expected_fail_skip.robot new file mode 100644 index 000000000..af4e4e23a --- /dev/null +++ b/robot/fixtures/tdd_expected_fail_skip.robot @@ -0,0 +1,14 @@ +*** Settings *** +Documentation Fixture: a tdd_expected_fail test that is skipped. +... Used by helper_tdd_tag_validation.py to verify that a +... skipped test is left unchanged by the listener (SKIP is +... not evidence of a fix). +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Bug 997 Skipped Expected Fail Test Stays Skip + [Tags] tdd_bug tdd_bug_997 tdd_expected_fail tdd_fixture + [Documentation] This test skips intentionally to verify the listener + ... leaves SKIP status unchanged for tdd_expected_fail tests. + Skip Skipping intentionally to test SKIP handling diff --git a/robot/fixtures/tdd_missing_tdd_bug.robot b/robot/fixtures/tdd_missing_tdd_bug.robot new file mode 100644 index 000000000..a3db23f88 --- /dev/null +++ b/robot/fixtures/tdd_missing_tdd_bug.robot @@ -0,0 +1,13 @@ +*** Settings *** +Documentation Fixture: a test with tdd_bug_N but missing tdd_bug. +... Used by helper_tdd_tag_validation.py to verify tag +... validation catches the missing tdd_bug tag. +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Missing TDD Bug Tag Causes Validation Error + [Tags] tdd_bug_123 tdd_fixture + [Documentation] This test has tdd_bug_123 but no tdd_bug tag. + ... The listener should detect this and force a validation failure. + Log This should not matter -- validation should fail first. diff --git a/robot/fixtures/tdd_normal_test.robot b/robot/fixtures/tdd_normal_test.robot new file mode 100644 index 000000000..498e89db8 --- /dev/null +++ b/robot/fixtures/tdd_normal_test.robot @@ -0,0 +1,14 @@ +*** Settings *** +Documentation Fixture: a normal test with no TDD tags. +... Used by helper_tdd_tag_validation.py to verify that +... normal tests are completely unaffected by the listener. +... This file must NOT be picked up by the main pabot runner -- +... it is excluded via the tdd_fixture tag. + +*** Test Cases *** +Normal Test Unaffected By Listener + [Tags] tdd_fixture + [Documentation] A normal test that should pass without interference + ... from the TDD expected-fail listener. + Log Normal test execution -- no TDD tags present. + Should Be True ${TRUE} diff --git a/robot/helper_tdd_tag_validation.py b/robot/helper_tdd_tag_validation.py index 7b218c656..93d6f19a7 100644 --- a/robot/helper_tdd_tag_validation.py +++ b/robot/helper_tdd_tag_validation.py @@ -1,22 +1,32 @@ -"""Helper script for tdd_tag_validation.robot integration tests. +"""Helper for ``tdd_tag_validation.robot`` — exercises TDD bug-capture tags. -Each subcommand is a self-contained check that exercises the TDD -bug-capture tag validation and result-inversion logic defined in -``features/environment.py``. Sentinels are printed on success so -the Robot test case can assert correct behaviour. +Behave-side sub-commands (``validate_tags_*``, ``should_invert_*``, +``inversion_*``) test the logic in ``features/environment.py``. +Robot-side sub-commands run fixture ``.robot`` files as sub-processes +with the ``tdd_expected_fail_listener`` and inspect the output XML. + +Exit 0 = check passed, 1 = unexpected outcome. +See CONTRIBUTING.md > TDD Bug Test Tags. """ from __future__ import annotations +__all__: list[str] = [] + import sys +from collections.abc import Callable from pathlib import Path -from unittest.mock import MagicMock # Ensure the project root is importable so ``features.environment`` resolves. _ROOT = str(Path(__file__).resolve().parents[1]) if _ROOT not in sys.path: sys.path.insert(0, _ROOT) +_ROBOT_DIR = str(Path(__file__).resolve().parent) +if _ROBOT_DIR not in sys.path: + sys.path.insert(0, _ROBOT_DIR) + +from _tdd_fixture_runner import run_fixture, run_multi_fixture # noqa: E402 from behave.model import Status # noqa: E402 from features.environment import ( # noqa: E402 _UNEXPECTED_PASS_MSG, @@ -24,47 +34,10 @@ from features.environment import ( # noqa: E402 should_invert_result, validate_tdd_tags, ) - -# --------------------------------------------------------------------------- -# Mock helpers -# --------------------------------------------------------------------------- +from features.mocks.tdd_test_helpers import make_mock_scenario # noqa: E402 -def _make_mock_scenario( - tags: list[str], - steps_passed: bool = True, - hook_failed: bool = False, - was_dry_run: bool = False, - step_exception: BaseException | None = None, -) -> MagicMock: - """Build a lightweight mock ``Scenario`` for ``apply_tdd_inversion``.""" - scenario = MagicMock() - scenario.effective_tags = tags - scenario.name = "mock-scenario" - scenario.hook_failed = hook_failed - scenario.was_dry_run = was_dry_run - - mock_step = MagicMock() - if steps_passed: - mock_step.status = Status.passed - mock_step.exception = None - else: - mock_step.status = Status.failed - mock_step.exception = ( - step_exception - if step_exception is not None - else AssertionError("simulated assertion failure") - ) - scenario.all_steps = [mock_step] - return scenario - - -# --------------------------------------------------------------------------- -# Subcommands — validate_tdd_tags -# --------------------------------------------------------------------------- - - -def validate_tags_valid_combos() -> None: +def validate_tags_valid_combos() -> int: """Verify valid tag combinations raise no error.""" valid_sets: list[set[str]] = [ {"tdd_bug", "tdd_bug_42"}, @@ -81,101 +54,95 @@ def validate_tags_valid_combos() -> None: f"FAIL: valid tag set {tag_set} raised ValueError: {exc}", file=sys.stderr, ) - sys.exit(1) + return 1 print("validate-tags-valid-combos-ok") + return 0 -def validate_tags_bug_n_without_bug() -> None: +def validate_tags_bug_n_without_bug() -> int: """Verify @tdd_bug_ without @tdd_bug raises ValueError.""" try: validate_tdd_tags({"tdd_bug_42"}) except ValueError: print("validate-tags-bug-n-without-bug-ok") - return + return 0 print("FAIL: expected ValueError for tdd_bug_42 without tdd_bug", file=sys.stderr) - sys.exit(1) + return 1 -def validate_tags_expected_fail_missing_bug() -> None: +def validate_tags_expected_fail_missing_bug() -> int: """Verify @tdd_expected_fail without @tdd_bug raises ValueError.""" try: validate_tdd_tags({"tdd_expected_fail", "tdd_bug_42"}) except ValueError: print("validate-tags-expected-fail-missing-bug-ok") - return + return 0 print( "FAIL: expected ValueError for tdd_expected_fail without tdd_bug", file=sys.stderr, ) - sys.exit(1) + return 1 -def validate_tags_expected_fail_missing_bug_n() -> None: +def validate_tags_expected_fail_missing_bug_n() -> int: """Verify @tdd_expected_fail without @tdd_bug_ raises ValueError.""" try: validate_tdd_tags({"tdd_expected_fail", "tdd_bug"}) except ValueError: print("validate-tags-expected-fail-missing-bug-n-ok") - return + return 0 print( "FAIL: expected ValueError for tdd_expected_fail without tdd_bug_", file=sys.stderr, ) - sys.exit(1) + return 1 -# --------------------------------------------------------------------------- -# Subcommands — should_invert_result -# --------------------------------------------------------------------------- - - -def should_invert_with_expected_fail() -> None: +def should_invert_with_expected_fail() -> int: """Verify should_invert_result returns True when @tdd_expected_fail present.""" result = should_invert_result({"tdd_bug", "tdd_bug_42", "tdd_expected_fail"}) if result is not True: print(f"FAIL: expected True, got {result}", file=sys.stderr) - sys.exit(1) + return 1 print("should-invert-with-expected-fail-ok") + return 0 -def should_not_invert_without_expected_fail() -> None: +def should_not_invert_without_expected_fail() -> int: """Verify should_invert_result returns False without @tdd_expected_fail.""" result = should_invert_result({"tdd_bug", "tdd_bug_42"}) if result is not False: print(f"FAIL: expected False, got {result}", file=sys.stderr) - sys.exit(1) + return 1 print("should-not-invert-without-expected-fail-ok") + return 0 -# --------------------------------------------------------------------------- -# Subcommands — apply_tdd_inversion -# --------------------------------------------------------------------------- - - -def inversion_expected_fail() -> None: +def inversion_expected_fail() -> int: """Verify expected failure (failed=True) is inverted to passed.""" - scenario = _make_mock_scenario( + scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_42", "tdd_expected_fail"], steps_passed=False, ) result = apply_tdd_inversion(scenario, failed=True) if result is not False: print(f"FAIL: expected False (inverted), got {result}", file=sys.stderr) - sys.exit(1) + return 1 scenario.set_status.assert_called_with(Status.passed) print("inversion-expected-fail-ok") + return 0 -def inversion_unexpected_pass() -> None: +def inversion_unexpected_pass() -> int: """Verify unexpected pass (failed=False) is inverted to failure.""" - scenario = _make_mock_scenario( + scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_42", "tdd_expected_fail"], steps_passed=True, ) result = apply_tdd_inversion(scenario, failed=False) if result is not True: print(f"FAIL: expected True (forced failure), got {result}", file=sys.stderr) - sys.exit(1) + return 1 scenario.set_status.assert_called_with(Status.failed) # Verify synthetic error is attached to last step last_step = scenario.all_steps[-1] @@ -184,22 +151,23 @@ def inversion_unexpected_pass() -> None: f"FAIL: last step status should be failed, got {last_step.status}", file=sys.stderr, ) - sys.exit(1) + return 1 if not isinstance(last_step.exception, AssertionError): print( f"FAIL: expected AssertionError, got {type(last_step.exception)}", file=sys.stderr, ) - sys.exit(1) + return 1 if str(last_step.exception) != _UNEXPECTED_PASS_MSG: print(f"FAIL: unexpected message: {last_step.exception}", file=sys.stderr) - sys.exit(1) + return 1 print("inversion-unexpected-pass-ok") + return 0 -def inversion_hook_error_guard() -> None: +def inversion_hook_error_guard() -> int: """Verify hook_failed prevents inversion.""" - scenario = _make_mock_scenario( + scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_42", "tdd_expected_fail"], steps_passed=False, hook_failed=True, @@ -207,13 +175,14 @@ def inversion_hook_error_guard() -> None: result = apply_tdd_inversion(scenario, failed=True) if result is not True: print(f"FAIL: expected True (not inverted), got {result}", file=sys.stderr) - sys.exit(1) + return 1 print("inversion-hook-error-guard-ok") + return 0 -def inversion_dry_run_guard() -> None: +def inversion_dry_run_guard() -> int: """Verify dry-run mode prevents inversion.""" - scenario = _make_mock_scenario( + scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_42", "tdd_expected_fail"], steps_passed=True, was_dry_run=True, @@ -221,13 +190,14 @@ def inversion_dry_run_guard() -> None: result = apply_tdd_inversion(scenario, failed=False) if result is not False: print(f"FAIL: expected False (not inverted), got {result}", file=sys.stderr) - sys.exit(1) + return 1 print("inversion-dry-run-guard-ok") + return 0 -def inversion_non_assertion_guard() -> None: +def inversion_non_assertion_guard() -> int: """Verify non-AssertionError exceptions prevent inversion.""" - scenario = _make_mock_scenario( + scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_42", "tdd_expected_fail"], steps_passed=False, step_exception=RuntimeError("connection lost"), @@ -235,30 +205,257 @@ def inversion_non_assertion_guard() -> None: result = apply_tdd_inversion(scenario, failed=True) if result is not True: print(f"FAIL: expected True (not inverted), got {result}", file=sys.stderr) - sys.exit(1) + return 1 print("inversion-non-assertion-guard-ok") + return 0 -def inversion_no_tag_passthrough() -> None: +def inversion_no_tag_passthrough() -> int: """Verify scenarios without @tdd_expected_fail are not modified.""" - scenario = _make_mock_scenario( + scenario = make_mock_scenario( tags=["tdd_bug", "tdd_bug_42"], steps_passed=False, ) result = apply_tdd_inversion(scenario, failed=True) if result is not True: print(f"FAIL: expected True (passthrough), got {result}", file=sys.stderr) - sys.exit(1) + return 1 # set_status should NOT have been called scenario.set_status.assert_not_called() print("inversion-no-tag-passthrough-ok") + return 0 -# --------------------------------------------------------------------------- -# Dispatch -# --------------------------------------------------------------------------- +def cmd_expected_fail_inverted() -> int: + """Verify that a tdd_expected_fail test that fails is inverted to PASS.""" + status, message = run_fixture("tdd_expected_fail_fails") + if status == "PASS" and "TDD expected failure" in message: + print("tdd-expected-fail-inverted-ok") + return 0 + print( + f"FAIL: Expected PASS with 'TDD expected failure' message " + f"but got {status}. Message: {message}", + file=sys.stderr, + ) + return 1 -_COMMANDS = { + +def cmd_unexpected_pass_inverted() -> int: + """Verify that a tdd_expected_fail test that passes is inverted to FAIL.""" + status, message = run_fixture("tdd_expected_fail_passes") + if status == "FAIL" and "Bug appears to be fixed" in message: + print("tdd-unexpected-pass-inverted-ok") + return 0 + print( + f"FAIL: Expected FAIL with guidance but got {status}. Message: {message}", + file=sys.stderr, + ) + return 1 + + +def cmd_missing_tdd_bug_validation() -> int: + """Verify that tdd_bug_N without tdd_bug causes a validation error.""" + status, message = run_fixture("tdd_missing_tdd_bug") + if status == "FAIL" and "missing the required tdd_bug tag" in message: + print("tdd-missing-tdd-bug-validation-ok") + return 0 + print( + f"FAIL: Expected FAIL with 'missing the required tdd_bug tag' " + f"but got {status}. Message: {message}", + file=sys.stderr, + ) + return 1 + + +def cmd_missing_bug_n_validation() -> int: + """Verify that tdd_expected_fail without tdd_bug_N causes validation error.""" + status, message = run_fixture("tdd_expected_fail_missing_bug_n") + if status == "FAIL" and "tdd_bug_" in message: + print("tdd-missing-bug-n-validation-ok") + return 0 + print( + f"FAIL: Expected FAIL with validation error mentioning " + f"tdd_bug_ but got {status}. Message: {message}", + file=sys.stderr, + ) + return 1 + + +def cmd_normal_test_unaffected() -> int: + """Verify that a normal test (no TDD tags) is unaffected by a loaded listener. + + Runs the normal-test fixture together with a tdd_expected_fail fixture + in a single Robot invocation. This proves the listener is loaded + (the expected-fail fixture is inverted) but does NOT modify the normal + test. + """ + results = run_multi_fixture( + "tdd_normal_test", + "tdd_expected_fail_fails", + ) + + # The normal test must be PASS. + normal_name = "Normal Test Unaffected By Listener" + if normal_name not in results: + print( + f"FAIL: Normal test '{normal_name}' not found in results. " + f"Available: {list(results.keys())}", + file=sys.stderr, + ) + return 1 + normal_status, normal_msg = results[normal_name] + if normal_status != "PASS": + print( + f"FAIL: Expected normal test PASS but got {normal_status}. " + f"Message: {normal_msg}", + file=sys.stderr, + ) + return 1 + + # The expected-fail fixture must have been inverted to PASS (proving + # the listener is loaded and actively processing). + inverted_name = "Bug 999 Expected Failure Is Inverted To Pass" + if inverted_name not in results: + print( + f"FAIL: Expected-fail fixture '{inverted_name}' not found. " + f"Available: {list(results.keys())}", + file=sys.stderr, + ) + return 1 + inv_status, _ = results[inverted_name] + if inv_status != "PASS": + print( + f"FAIL: Expected-fail fixture was NOT inverted to PASS " + f"(got {inv_status}). Listener may not be loaded.", + file=sys.stderr, + ) + return 1 + + print("tdd-normal-test-unaffected-ok") + return 0 + + +def cmd_skip_status_unchanged() -> int: + """Verify that a skipped tdd_expected_fail test stays SKIP. + + Runs alongside a tdd_expected_fail fixture to prove the listener + is loaded and selectively applies — the SKIP test must stay SKIP + while the companion is correctly inverted to PASS. + """ + results = run_multi_fixture( + "tdd_expected_fail_skip", + "tdd_expected_fail_fails", + ) + + # The SKIP test must remain SKIP. + skip_name = "Bug 997 Skipped Expected Fail Test Stays Skip" + if skip_name not in results: + print( + f"FAIL: Skip fixture '{skip_name}' not found. " + f"Available: {list(results.keys())}", + file=sys.stderr, + ) + return 1 + skip_status, skip_msg = results[skip_name] + if skip_status != "SKIP": + print( + f"FAIL: Expected SKIP but got {skip_status}. Message: {skip_msg}", + file=sys.stderr, + ) + return 1 + + # The expected-fail companion must be inverted to PASS (proving + # the listener is loaded and actively processing). + inverted_name = "Bug 999 Expected Failure Is Inverted To Pass" + if inverted_name not in results: + print( + f"FAIL: Companion fixture '{inverted_name}' not found. " + f"Available: {list(results.keys())}", + file=sys.stderr, + ) + return 1 + inv_status, _ = results[inverted_name] + if inv_status != "PASS": + print( + f"FAIL: Companion fixture was NOT inverted to PASS " + f"(got {inv_status}). Listener may not be loaded.", + file=sys.stderr, + ) + return 1 + + print("tdd-skip-status-unchanged-ok") + return 0 + + +def cmd_expected_fail_alone_validation() -> int: + """Verify tdd_expected_fail alone (no tdd_bug or tdd_bug_N) fails validation.""" + status, message = run_fixture("tdd_expected_fail_alone") + if status == "FAIL" and "tdd_bug" in message and "tdd_bug_" in message: + print("tdd-expected-fail-alone-validation-ok") + return 0 + print( + f"FAIL: Expected FAIL mentioning both tdd_bug and tdd_bug_ " + f"but got {status}. Message: {message}", + file=sys.stderr, + ) + return 1 + + +def cmd_tdd_bug_alone_valid() -> int: + """Verify tdd_bug alone (no tdd_bug_N or tdd_expected_fail) is valid. + + Runs alongside a tdd_expected_fail fixture to prove the listener + is loaded — the tdd_bug-alone test must stay PASS while the + companion is correctly inverted. + """ + results = run_multi_fixture( + "tdd_bug_alone", + "tdd_expected_fail_fails", + ) + + # The tdd_bug-alone test must be PASS (listener should not modify). + alone_name = "TDD Bug Tag Alone Is Valid" + if alone_name not in results: + print( + f"FAIL: tdd_bug-alone fixture '{alone_name}' not found. " + f"Available: {list(results.keys())}", + file=sys.stderr, + ) + return 1 + alone_status, alone_msg = results[alone_name] + if alone_status != "PASS": + print( + f"FAIL: Expected PASS for tdd_bug-alone test but got " + f"{alone_status}. Message: {alone_msg}", + file=sys.stderr, + ) + return 1 + + # The expected-fail companion must be inverted to PASS (proving + # the listener is loaded and actively processing). + inverted_name = "Bug 999 Expected Failure Is Inverted To Pass" + if inverted_name not in results: + print( + f"FAIL: Companion fixture '{inverted_name}' not found. " + f"Available: {list(results.keys())}", + file=sys.stderr, + ) + return 1 + inv_status, _ = results[inverted_name] + if inv_status != "PASS": + print( + f"FAIL: Companion fixture was NOT inverted to PASS " + f"(got {inv_status}). Listener may not be loaded.", + file=sys.stderr, + ) + return 1 + + print("tdd-bug-alone-valid-ok") + return 0 + + +_COMMANDS: dict[str, Callable[[], int]] = { + # Behave-side commands "validate_tags_valid_combos": validate_tags_valid_combos, "validate_tags_bug_n_without_bug": validate_tags_bug_n_without_bug, "validate_tags_expected_fail_missing_bug": validate_tags_expected_fail_missing_bug, @@ -273,11 +470,30 @@ _COMMANDS = { "inversion_dry_run_guard": inversion_dry_run_guard, "inversion_non_assertion_guard": inversion_non_assertion_guard, "inversion_no_tag_passthrough": inversion_no_tag_passthrough, + # Robot-side commands + "expected-fail-inverted": cmd_expected_fail_inverted, + "unexpected-pass-inverted": cmd_unexpected_pass_inverted, + "missing-tdd-bug-validation": cmd_missing_tdd_bug_validation, + "missing-bug-n-validation": cmd_missing_bug_n_validation, + "normal-test-unaffected": cmd_normal_test_unaffected, + "skip-status-unchanged": cmd_skip_status_unchanged, + "expected-fail-alone-validation": cmd_expected_fail_alone_validation, + "tdd-bug-alone-valid": cmd_tdd_bug_alone_valid, } -if __name__ == "__main__": + +def main() -> None: + """Dispatch to the requested sub-command.""" if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: - print(f"Usage: {sys.argv[0]} ", file=sys.stderr) - print(f"Commands: {list(_COMMANDS)}", file=sys.stderr) + print( + f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", + file=sys.stderr, + ) sys.exit(1) - _COMMANDS[sys.argv[1]]() + + handler = _COMMANDS[sys.argv[1]] + sys.exit(handler()) + + +if __name__ == "__main__": + main() diff --git a/robot/tdd_expected_fail_listener.py b/robot/tdd_expected_fail_listener.py index d89f9ced7..a66ece879 100644 --- a/robot/tdd_expected_fail_listener.py +++ b/robot/tdd_expected_fail_listener.py @@ -1,48 +1,262 @@ -"""Robot Framework listener that inverts results for ``tdd_expected_fail`` tests. +"""Robot Framework Listener v3 for TDD bug-capture tag handling. -When a test is tagged ``tdd_expected_fail``, the listener treats a FAIL as a -PASS (the bug still exists, which is expected) and a PASS as a FAIL (the bug -was fixed but the tag was not removed). +Implements the three-tag TDD bug-capture system for Robot Framework +integration tests, paralleling the Behave implementation in +``features/environment.py``. -See CONTRIBUTING.md § TDD Bug Test Tags for the full convention. +Three-Tag System (see CONTRIBUTING.md > TDD Bug Test Tags) +----------------------------------------------------------- +``tdd_bug`` + Generic filter tag. Present on ALL TDD bug tests. Used to list, + filter, and count TDD bug tests across the codebase (e.g., + ``--include tdd_bug``). -Usage:: +``tdd_bug_`` + Issue reference tag (e.g., ``tdd_bug_123``). Links the test to the + specific ``Type/Bug`` issue it captures. N is the bug issue number. + Permanent -- never removed. - pabot ... --listener robot/tdd_expected_fail_listener.py ... +``tdd_expected_fail`` + Behavioral switch. When present, the test result is inverted: -This listener uses the Robot Framework Listener API v3. + * A **failing** test (bug still exists) is reported as **passed**. + * A **passing** test (bug was fixed without removing the tag) is + reported as **failed** with a clear guidance message. + + Temporary -- removed by the bug-fix developer when the fix is + implemented. + +Tag Validation Rules +-------------------- +* ``tdd_bug_`` requires ``tdd_bug`` to also be present. +* ``tdd_expected_fail`` requires both ``tdd_bug`` and at least one + ``tdd_bug_``. + +Implementation Notes +-------------------- +The listener uses the Robot Framework Listener v3 API with module-level +``start_test`` and ``end_test`` functions. Tag validation happens in +``start_test``; result inversion happens in ``end_test``. Validation +errors are stored in ``_validation_errors`` keyed by test ``full_name`` +and applied in ``end_test`` to ensure the error message is visible in +reports. + +Tags are read from ``data.tags`` (the static test definition) rather +than ``result.tags``. This is intentional: TDD tags are declarative +metadata that must be present in the source file. Runtime tag +modification via ``Set Tags`` / ``Remove Tags`` keywords is not +supported for TDD tags — they must be statically declared. + +An idempotency guard (``_processed_tests``) prevents double-inversion +if the listener is loaded more than once (e.g., ``--listener`` specified +both in noxfile.py and via user arguments). + +Registration: The listener is registered via ``--listener`` in the nox +``integration_tests`` and ``slow_integration_tests`` sessions. """ from __future__ import annotations -from typing import Any +import logging +import re + +from robot.result.model import TestCase as ResultTestCase +from robot.running.model import TestCase as RunningTestCase ROBOT_LISTENER_API_VERSION = 3 -_TAG = "tdd_expected_fail" +_TDD_BUG_N_RE = re.compile(r"tdd_bug_\d+") + +_logger = logging.getLogger("cleveragents.testing.robot_tdd_tags") + +# Validation errors detected in start_test are stored here keyed by test +# longname so that end_test can override whatever result the test produced. +_validation_errors: dict[str, str] = {} + +# Idempotency guard: prevents double-invocation if the listener is loaded +# more than once in the same process (e.g., --listener specified twice or +# via both noxfile and user args). Each test is processed at most once. +_processed_tests: set[str] = set() -def end_test(data: Any, result: Any) -> None: - """Invert the result of tests tagged ``tdd_expected_fail``.""" - tags = getattr(result, "tags", []) - if _TAG not in tags: +def _validate_tdd_tags(tags: set[str]) -> str | None: + """Validate TDD bug-capture tag combinations. + + Returns an error message string if the tag set is inconsistent, or + ``None`` if the tags are valid. + + Design note: this function returns ``str | None`` rather than raising + ``ValueError`` (as the Behave counterpart ``validate_tdd_tags`` does) + because the Robot Framework Listener v3 API does not propagate + exceptions cleanly from ``start_test``. Returning a string allows + ``start_test`` to store the error for ``end_test`` to apply as a test + failure, giving better diagnostics in the Robot output XML. + + Rules (from CONTRIBUTING.md > TDD Bug Test Tags): + * ``tdd_bug_`` requires ``tdd_bug`` to also be present. + * ``tdd_expected_fail`` requires both ``tdd_bug`` and at least one + ``tdd_bug_``. + + Args: + tags: The tags on the test case (lowercased). + + Returns: + An error message string if validation fails, ``None`` otherwise. + """ + has_tdd_bug = "tdd_bug" in tags + has_tdd_bug_n = any(_TDD_BUG_N_RE.fullmatch(t) for t in tags) + has_expected_fail = "tdd_expected_fail" in tags + + # Note: error messages reference tag names without the ``@`` prefix used + # by Behave (e.g., ``tdd_bug`` not ``@tdd_bug``). Robot Framework tags + # do not use the ``@`` prefix — this divergence from the Behave error + # messages is intentional and matches each framework's convention. + if has_tdd_bug_n and not has_tdd_bug: + bug_n_tags = sorted(t for t in tags if _TDD_BUG_N_RE.fullmatch(t)) + return ( + f"Test has {', '.join(bug_n_tags)} but is missing the required " + f"tdd_bug tag. All TDD bug tests must include tdd_bug. " + f"See CONTRIBUTING.md > TDD Bug Test Tags." + ) + + if has_expected_fail: + missing: list[str] = [] + if not has_tdd_bug: + missing.append("tdd_bug") + if not has_tdd_bug_n: + missing.append("tdd_bug_") + if missing: + return ( + f"Test has tdd_expected_fail but is missing required " + f"tag(s): {', '.join(missing)}. tdd_expected_fail requires " + f"both tdd_bug and at least one tdd_bug_. " + f"See CONTRIBUTING.md > TDD Bug Test Tags." + ) + + return None + + +def _should_invert_result(tags: set[str]) -> bool: + """Return ``True`` if the test result should be inverted. + + A test result is inverted when ``tdd_expected_fail`` is present, + indicating the test captures a known bug that has not yet been fixed. + + Args: + tags: The tags on the test case (lowercased). + """ + return "tdd_expected_fail" in tags + + +def start_test( + data: RunningTestCase, + result: ResultTestCase, +) -> None: + """Validate TDD tags before the test runs. + + If validation fails, the error is stored for ``end_test`` to apply. + The test is allowed to execute so that ``end_test`` can properly + override the result. + """ + tags = {str(t).lower() for t in data.tags} + error = _validate_tdd_tags(tags) + if error is not None: + _validation_errors[result.full_name] = error + + +def end_test( + data: RunningTestCase, + result: ResultTestCase, +) -> None: + """Invert test results for ``tdd_expected_fail`` tests and apply + any tag validation errors. + + If there was a validation error from ``start_test``, the test is + forced to fail with the validation error message regardless of + the actual test outcome. + + For ``tdd_expected_fail`` tests with valid tags: + * If the test **failed** (bug still exists), the result is inverted + to **PASS** (expected failure). + * If the test **passed** (bug appears fixed), the result is inverted + to **FAIL** with a guidance message. + * If the test was **skipped** or has any other status, the result is + left unchanged (a skipped test is not evidence of a fix). + + An idempotency guard prevents double-processing if the listener is + loaded multiple times. + """ + full_name = result.full_name + + # Idempotency guard — skip if this test was already processed by + # a previous listener instance in the same process. + if full_name in _processed_tests: + return + _processed_tests.add(full_name) + + # --- Handle validation errors first --- + if full_name in _validation_errors: + error_msg = _validation_errors.pop(full_name) + result.status = "FAIL" + result.message = f"TDD tag validation error: {error_msg}" + _logger.error( + "TDD tag validation error in '%s': %s", + full_name, + error_msg, + ) return - status: str = getattr(result, "status", "") - original_message: str = getattr(result, "message", "") + # Tags are read from ``data.tags`` (static test definition) rather + # than ``result.tags``. TDD tags are declarative metadata and must + # be present in the source file — runtime ``Set Tags`` / ``Remove + # Tags`` modifications are not supported for TDD tags. + tags = {str(t).lower() for t in data.tags} + if not _should_invert_result(tags): + return - if status == "FAIL": - # Expected failure — bug still exists. Mark as PASS. + if result.status == "FAIL": + # Expected failure -- the bug still exists. Invert to PASS. + _logger.info( + "TDD expected failure: test '%s' failed as expected " + "(bug still exists). Inverting to PASS.", + full_name, + ) result.status = "PASS" result.message = ( - f"[tdd_expected_fail] Expected failure (bug still present). " - f"Original: {original_message}" + "TDD expected failure: test failed as expected (bug still exists)." + ) + elif result.status == "PASS": + # Unexpected pass -- the bug appears to be fixed but the + # tdd_expected_fail tag has not been removed. + guidance = ( + "Bug appears to be fixed. Remove the tdd_expected_fail tag " + "from this test and verify the fix through the bug fix " + "workflow. See CONTRIBUTING.md > Bug Fix Workflow." + ) + _logger.warning( + "%s Test: '%s'", + guidance, + full_name, ) - elif status == "PASS": - # Unexpected pass — bug appears fixed, tag should be removed. result.status = "FAIL" - result.message = ( - "[tdd_expected_fail] Test passed but still has the " - "tdd_expected_fail tag. The bug appears to be fixed — " - "remove the tag." + result.message = guidance + else: + # SKIP or other statuses — leave unchanged. A skipped + # tdd_expected_fail test is not evidence of a fix; it simply + # did not execute. + _logger.debug( + "TDD expected-fail test '%s' has status %s — leaving unchanged.", + full_name, + result.status, ) + + +def close() -> None: + """Clean up module-level state when the listener is unloaded. + + Called by Robot Framework when test execution finishes. Clears + internal state so the listener can be reused in long-lived processes + or consecutive Robot runs without stale data. + """ + _validation_errors.clear() + _processed_tests.clear() diff --git a/robot/tdd_tag_validation.robot b/robot/tdd_tag_validation.robot index dc58729a7..18e0f4f69 100644 --- a/robot/tdd_tag_validation.robot +++ b/robot/tdd_tag_validation.robot @@ -1,9 +1,16 @@ *** Settings *** Documentation Integration tests for TDD bug-capture tag validation and -... @tdd_expected_fail result inversion logic defined in -... features/environment.py. Covers validate_tdd_tags(), -... should_invert_result(), and apply_tdd_inversion() including -... all guard paths (hook errors, dry-run, non-assertion exceptions). +... result-inversion logic. Covers both the Behave implementation +... (``features/environment.py``: ``validate_tdd_tags()``, +... ``should_invert_result()``, ``apply_tdd_inversion()``) and the +... Robot Framework listener (``tdd_expected_fail_listener.py``). +... +... Robot-side tests run fixture ``.robot`` files via a helper +... script that launches Robot with the listener active, then +... checks the final test status in the output XML. +... +... See CONTRIBUTING.md > TDD Bug Test Tags for the full +... specification. Resource ${CURDIR}/common.resource Suite Setup Setup Test Environment Suite Teardown Cleanup Test Environment @@ -12,6 +19,10 @@ Suite Teardown Cleanup Test Environment ${HELPER} ${CURDIR}/helper_tdd_tag_validation.py *** Test Cases *** +# =========================================================================== +# Behave-side tests (features/environment.py) +# =========================================================================== + Valid Tag Combinations Are Accepted [Documentation] Verify valid TDD tag sets raise no errors [Tags] testing tdd validation @@ -132,3 +143,90 @@ Inversion No Tag Passthrough Log ${result.stderr} Should Be Equal As Integers ${result.rc} 0 Should Contain ${result.stdout} inversion-no-tag-passthrough-ok + +# =========================================================================== +# Robot-side tests (tdd_expected_fail_listener.py) +# =========================================================================== + +TDD Expected Fail Test That Fails Is Inverted To Pass + [Documentation] A test tagged ``tdd_expected_fail`` that deliberately fails + ... should have its result inverted to PASS by the listener (bug still exists). + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} expected-fail-inverted cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-expected-fail-inverted-ok + +TDD Expected Fail Test That Passes Is Inverted To Fail + [Documentation] A test tagged ``tdd_expected_fail`` that unexpectedly passes + ... should have its result inverted to FAIL with a guidance message telling the + ... developer to remove the tag. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} unexpected-pass-inverted cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-unexpected-pass-inverted-ok + +TDD Tag Validation Catches Missing tdd_bug + [Documentation] A test with ``tdd_bug_`` but missing ``tdd_bug`` should + ... fail with a clear validation error message. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} missing-tdd-bug-validation cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-missing-tdd-bug-validation-ok + +TDD Tag Validation Catches Missing tdd_bug_N + [Documentation] A test with ``tdd_expected_fail`` and ``tdd_bug`` but missing + ... ``tdd_bug_`` should fail with a clear validation error message. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} missing-bug-n-validation cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-missing-bug-n-validation-ok + +Normal Test Unaffected By TDD Listener + [Documentation] A test with no TDD tags run alongside a tdd_expected_fail test + ... should be completely unaffected by the listener. Proves the listener is + ... loaded (the companion fixture is inverted) but does not modify normal tests. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} normal-test-unaffected cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-normal-test-unaffected-ok + +TDD Expected Fail Skip Status Is Unchanged + [Documentation] A ``tdd_expected_fail`` test that is skipped should stay SKIP. + ... The listener should not treat a skipped test as evidence of a fix. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} skip-status-unchanged cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-skip-status-unchanged-ok + +TDD Tag Validation Catches Expected Fail Alone + [Documentation] A test with only ``tdd_expected_fail`` (both ``tdd_bug`` and + ... ``tdd_bug_`` missing) should fail with validation error mentioning both. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} expected-fail-alone-validation cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-expected-fail-alone-validation-ok + +TDD Bug Tag Alone Is Valid + [Documentation] A test with only ``tdd_bug`` (no ``tdd_bug_`` or + ... ``tdd_expected_fail``) should pass normally — the listener should + ... not interfere. ``tdd_bug`` alone is a valid tag combination. + [Tags] tdd_infrastructure + ${result}= Run Process ${PYTHON} ${HELPER} tdd-bug-alone-valid cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-bug-alone-valid-ok -- 2.52.0