Add deterministic fixture manifest#129
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized fixtures/manifest.json to manage fixture metadata and updates documentation to require manifest registration for all fixture bundles. A comprehensive test suite is added to validate manifest integrity, including path existence, metadata consistency, and degradation level uniqueness. Review feedback identifies a redundant json.dumps call that should be removed and suggests using Path.as_posix() for more idiomatic path normalization.
|
|
||
| def test_manifest_is_json_serializable_and_sorted() -> None: | ||
| manifest = _load_manifest() | ||
| json.dumps(manifest, sort_keys=True) |
There was a problem hiding this comment.
The call to json.dumps(manifest, sort_keys=True) is redundant. Since the manifest object was just loaded from a JSON file using json.load (via _load_manifest), it is already guaranteed to be serializable. Additionally, the result of the call is not assigned or asserted against, making this a no-op. If the intention was to verify the formatting or key ordering of the manifest file on disk, this line does not achieve that.
| registered_paths = {entry["path"] for entry in manifest["fixtures"]} | ||
|
|
||
| discovered_fixture_paths = { | ||
| str(path.parent.parent.relative_to(ROOT)).replace("\\", "/") |
There was a problem hiding this comment.
Using str(...).replace("\\", "/") for path normalization is less idiomatic in modern Python. The Path.as_posix() method is the standard way to obtain a string representation with forward slashes, ensuring consistent behavior across different operating systems.
| str(path.parent.parent.relative_to(ROOT)).replace("\\", "/") | |
| path.parent.parent.relative_to(ROOT).as_posix() |
Motivation
Description
fixtures/manifest.jsonthat registers all current coding-workflow fixture bundles with stable ordering, lexicographically-sortedcontractsandexpected_failure_labels, and no timestamps or environment-dependent fields.fixtures/manifest.json.fixtures/manifest.json,tests/test_fixture_manifest.py,docs/FIXTURE_TEMPLATE_v1.md,docs/benchmarks/layered_admissibility.md.tests/test_fixture_manifest.pyimplementing the 8 required checks (manifest serializability and ordering, path existence, admissibility metadata alignment, contract files matching, failures alignment, benchmark artifact references, degradation-level constraints, and discovery of unregistered fixture directories).baseline,mild,moderate,severe),contractsandexpected_failure_labelsare lexicographically sorted, and there are no generated timestamps or environment-dependent values.Testing
pytest tests/test_fixture_manifest.py -q— 8 passed.pytest tests/test_degradation_curve_generator.py -q— 12 passed;pytest tests/test_svg_curve_renderer.py -q— 7 passed.pytest -q— 192 passed.npm run check— completed successfully (includes layout/typecheck/validate/build/test steps).Summary:
fixtures/manifest.json, a comprehensive manifest validation test suite, and brief docs updates to require manifest registration.Changed files:
fixtures/manifest.jsontests/test_fixture_manifest.pydocs/FIXTURE_TEMPLATE_v1.mddocs/benchmarks/layered_admissibility.mdTesting:
pytest tests/test_fixture_manifest.py -q(passed)pytest tests/test_degradation_curve_generator.py -q(passed)pytest tests/test_svg_curve_renderer.py -q(passed)pytest -q(all tests passed)npm run check(passed)Risks:
coding_workflowfixture family; additional families must be added explicitly.Next:
Codex Task