diff --git a/tests/claude_code/_builder_unit_tests/test_matrix_builder.py b/tests/claude_code/_builder_unit_tests/test_matrix_builder.py index 17411464050..2644e092e48 100644 --- a/tests/claude_code/_builder_unit_tests/test_matrix_builder.py +++ b/tests/claude_code/_builder_unit_tests/test_matrix_builder.py @@ -90,6 +90,105 @@ def test_build_matrix_any_fail_makes_cell_fail(): assert cell["error"] == "[claude-opus-4-7] timeout" +def test_build_matrix_joins_all_failure_errors_in_one_cell(): + """When multiple tiers fail for different reasons within the same cell, + every failure's error must appear in the published cell so triage + isn't reduced to a single tier's diagnostic. + """ + manifest = { + "schema_version": "1", + "providers": ["anthropic"], + "features": [{"id": "f", "name": "F"}], + } + results = [ + { + "feature_id": "f", + "provider": "anthropic", + "result": {"status": "fail", "error": "[claude-haiku-4-5] 429"}, + }, + {"feature_id": "f", "provider": "anthropic", "result": {"status": "pass"}}, + { + "feature_id": "f", + "provider": "anthropic", + "result": {"status": "fail", "error": "[claude-opus-4-7] timeout"}, + }, + ] + matrix = build_matrix( + manifest=manifest, + results=results, + litellm_version="v", + claude_code_version="c", + generated_at="t", + ) + cell = matrix["features"][0]["providers"]["anthropic"] + assert cell["status"] == "fail" + assert "[claude-haiku-4-5] 429" in cell["error"] + assert "[claude-opus-4-7] timeout" in cell["error"] + + +def test_build_matrix_mixed_pass_and_not_tested_surfaces_pass(): + """A `not_tested` row mixed with `pass` rows must not silently demote + the cell to `not_tested` — `not_tested` is "absent data", not a + negative signal. Otherwise a partial crash mid-test, or a test that + explicitly recorded "tier didn't run", would discard real passing + results from the published cell. + """ + manifest = { + "schema_version": "1", + "providers": ["anthropic"], + "features": [{"id": "f", "name": "F"}], + } + results = [ + {"feature_id": "f", "provider": "anthropic", "result": {"status": "pass"}}, + { + "feature_id": "f", + "provider": "anthropic", + "result": {"status": "not_tested"}, + }, + {"feature_id": "f", "provider": "anthropic", "result": {"status": "pass"}}, + ] + matrix = build_matrix( + manifest=manifest, + results=results, + litellm_version="v", + claude_code_version="c", + generated_at="t", + ) + assert matrix["features"][0]["providers"]["anthropic"] == {"status": "pass"} + + +def test_build_matrix_all_not_tested_stays_not_tested(): + """A cell whose every row is `not_tested` (or empty) must remain + `not_tested` — the absent-data rule only drops `not_tested` rows + when there's other signal to surface. + """ + manifest = { + "schema_version": "1", + "providers": ["anthropic"], + "features": [{"id": "f", "name": "F"}], + } + results = [ + { + "feature_id": "f", + "provider": "anthropic", + "result": {"status": "not_tested"}, + }, + { + "feature_id": "f", + "provider": "anthropic", + "result": {"status": "not_tested"}, + }, + ] + matrix = build_matrix( + manifest=manifest, + results=results, + litellm_version="v", + claude_code_version="c", + generated_at="t", + ) + assert matrix["features"][0]["providers"]["anthropic"] == {"status": "not_tested"} + + def test_build_matrix_fills_not_tested_for_missing_cells(): manifest = { "schema_version": "1", diff --git a/tests/claude_code/_builder_unit_tests/test_v0_layout.py b/tests/claude_code/_builder_unit_tests/test_v0_layout.py index 459db6cb153..b1745008fac 100644 --- a/tests/claude_code/_builder_unit_tests/test_v0_layout.py +++ b/tests/claude_code/_builder_unit_tests/test_v0_layout.py @@ -46,6 +46,23 @@ EXPECTED_PROVIDERS = [ ] +def _all_manifest_feature_ids() -> list[str]: + """Every feature_id currently declared in `manifest.yaml`. + + Evaluated at import time so the result can drive parametrized + structural tests below. Used to catch layout drift on post-v0 + feature rows added after the matrix shipped — the v0 anchor + constants above only validate the original six rows by design. + """ + return [ + feature["id"] + for feature in yaml.safe_load(MANIFEST_PATH.read_text())["features"] + ] + + +ALL_FEATURE_IDS = _all_manifest_feature_ids() + + @pytest.fixture(scope="module") def manifest() -> dict: return yaml.safe_load(MANIFEST_PATH.read_text()) @@ -93,6 +110,37 @@ def test_feature_directory_has_init_file(feature_id): assert init_file.is_file(), f"missing __init__.py: {init_file}" +# Manifest-driven structural tests: every feature in `manifest.yaml` +# (v0 and post-v0 alike) must have the expected on-disk layout. The +# v0-only tests above pin the position of the original six rows; these +# extend the same structural guarantees to any row added afterward so +# a broken post-v0 directory still fails CI. +@pytest.mark.parametrize("feature_id", ALL_FEATURE_IDS) +def test_every_manifest_feature_has_directory(feature_id): + feature_dir = REPO_ROOT / feature_id + assert feature_dir.is_dir(), ( + f"manifest declares {feature_id!r} but {feature_dir} is missing — " + "feature_id MUST match its on-disk directory (see manifest.yaml header)." + ) + + +@pytest.mark.parametrize("feature_id", ALL_FEATURE_IDS) +def test_every_manifest_feature_has_init_file(feature_id): + init_file = REPO_ROOT / feature_id / "__init__.py" + assert init_file.is_file(), f"missing __init__.py: {init_file}" + + +@pytest.mark.parametrize("feature_id", ALL_FEATURE_IDS) +@pytest.mark.parametrize("provider", EXPECTED_PROVIDERS) +def test_every_manifest_feature_has_per_provider_test_file(feature_id, provider): + """Every (feature, provider) cell in the rendered matrix must be + backed by a per-provider test file. Without this check, a missing + file silently becomes a `not_tested` cell in the published matrix + rather than a CI failure surfacing the layout drift.""" + test_file = REPO_ROOT / feature_id / f"test_{provider}.py" + assert test_file.is_file(), f"missing per-provider test file: {test_file}" + + @pytest.mark.parametrize("feature_id", EXPECTED_FEATURE_IDS) @pytest.mark.parametrize("provider", EXPECTED_PROVIDERS) def test_per_provider_test_file_imports_and_parametrizes_three_models( diff --git a/tests/claude_code/matrix_builder.py b/tests/claude_code/matrix_builder.py index b9bf8ecc598..8555805dab2 100644 --- a/tests/claude_code/matrix_builder.py +++ b/tests/claude_code/matrix_builder.py @@ -130,26 +130,40 @@ def _aggregate_cell(results: Sequence[Mapping[str, Any]]) -> Dict[str, Any]: """Aggregate a list of per-model results into a single cell status. Order of precedence (most informative wins): - - Any `fail` → cell is `fail` with the first failure's error. + - Any `fail` → cell is `fail` with every failing model's error + joined by `"; "` so a multi-tier breakage doesn't silently hide + all but the first error from the published matrix. - `not_applicable` → cell is `not_applicable` with the reason. - `pass` → cell is `pass`. - empty / nothing recognized → `not_tested`. + + `not_tested` rows are treated as absent data: they're dropped before + aggregation so a mix of (pass, not_tested) — e.g. from a partial + crash or a test that explicitly recorded "this tier didn't run" — + still surfaces the passing tiers rather than silently demoting the + whole cell to `not_tested`. A cell is only `not_tested` when *every* + row is `not_tested` (or there are no rows at all). """ if not results: return {"status": "not_tested"} - for r in results: - if r.get("status") == "fail": - return {"status": "fail", "error": str(r.get("error", "test failed"))} + observed = [r for r in results if r.get("status") != "not_tested"] + if not observed: + return {"status": "not_tested"} - for r in results: + failures = [r for r in observed if r.get("status") == "fail"] + if failures: + errors = [str(r.get("error", "test failed")) for r in failures] + return {"status": "fail", "error": "; ".join(errors)} + + for r in observed: if r.get("status") == "not_applicable": return { "status": "not_applicable", "reason": str(r.get("reason", "not applicable")), } - if all(r.get("status") == "pass" for r in results): + if all(r.get("status") == "pass" for r in observed): return {"status": "pass"} return {"status": "not_tested"}