test: add family-agnostic manifest fixture family validation coverage#134
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new test suite, tests/test_manifest_fixture_families.py, to ensure the structural and behavioral consistency of fixture families defined in the manifest. The tests verify that each family contains the necessary degradation levels and that the validation results for baseline and severe fixtures align with expectations. Feedback identifies opportunities to improve sorting robustness by using the defined REQUIRED_LEVELS order and suggests removing a redundant assertion.
| manifest = _load_json(MANIFEST_PATH) | ||
| fixtures = manifest["fixtures"] | ||
| assert isinstance(fixtures, list) | ||
| return sorted(fixtures, key=lambda entry: (str(entry["family"]), str(entry["degradation_level"]), str(entry["fixture_id"]))) |
There was a problem hiding this comment.
Sorting degradation_level alphabetically is brittle. This works for the current levels, but if a new level is added that doesn't fit the alphabetical order (e.g., critical), the sorting will be incorrect. It's more robust to sort based on the order defined in REQUIRED_LEVELS.
| return sorted(fixtures, key=lambda entry: (str(entry["family"]), str(entry["degradation_level"]), str(entry["fixture_id"]))) | |
| return sorted(fixtures, key=lambda entry: (str(entry["family"]), REQUIRED_LEVELS.index(str(entry["degradation_level"])), str(entry["fixture_id"]))) |
| grouped: dict[str, list[dict[str, object]]] = defaultdict(list) | ||
| for entry in entries: | ||
| grouped[str(entry["family"])].append(entry) | ||
| return {family: sorted(grouped[family], key=lambda entry: str(entry["degradation_level"])) for family in sorted(grouped)} |
There was a problem hiding this comment.
Similar to the sorting in _load_manifest_entries, sorting degradation_level alphabetically here is brittle. For robustness, it's better to use the explicit order from REQUIRED_LEVELS to ensure correctness if new levels are added in the future.
| return {family: sorted(grouped[family], key=lambda entry: str(entry["degradation_level"])) for family in sorted(grouped)} | |
| return {family: sorted(grouped[family], key=lambda entry: REQUIRED_LEVELS.index(str(entry["degradation_level"]))) for family in sorted(grouped)} |
| expected_failure_labels = sorted(severe["expected_failure_labels"]) | ||
| observed_failure_labels = sorted({result.failure_label for result in severe_results if result.failure_label is not None}) | ||
| assert observed_failure_labels == expected_failure_labels | ||
| assert set(observed_failure_labels).issubset(set(expected_failure_labels)) |
There was a problem hiding this comment.
This assertion is redundant because the check on the previous line (assert observed_failure_labels == expected_failure_labels) is stricter and already guarantees equality. If the sorted lists are identical, the set of observed labels is guaranteed to be a subset of the expected ones. This line can be safely removed.
Summary:
Add a new focused test module that validates fixture-family invariants and baseline/severe behavioral expectations across all fixture families registered in
fixtures/manifest.json.Changed files:
Testing:
pytest tests/test_manifest_fixture_families.py -q— passed (3 passed).pytest tests/test_fixture_manifest.py -q— passed (8 passed).pytest tests/test_incident_response_fixture_contract_bundle.py -q— passed (2 passed).pytest tests/test_degradation_curve_generator.py -q— passed (13 passed).npm run check— passed (full test suite: 202 passed).Risks:
ContractValidatorandAdmissibilityScorerand do not modify fixtures, manifest, or runtime logic.Next:
Codex Task