From 6ca7c16844ef0ebcc53bdaee7eb6db474ab39b75 Mon Sep 17 00:00:00 2001 From: David Hoffman Date: Sun, 11 Oct 2026 16:13:24 -0700 Subject: [PATCH] Add ASD test audit workflow and strengthen verification tests --- .agents/skills/test-audit/SKILL.md | 85 +++++++++++++ AGENTS.md | 2 + ...002-coordinator-isolate-negative-guards.md | 9 ++ .../DELIVERY-SYSTEM-SPEC.md | 2 +- .../REFERENCES.md | 2 + docs/tasks/TEST-AUDIT-002.md | 91 +++++++++++++ tests/test_verification.py | 120 +++++++++--------- tests/test_verification_hooks.py | 46 ++----- tests/test_verification_scope.py | 15 ++- tests/test_verification_shards.py | 26 ++-- 10 files changed, 292 insertions(+), 106 deletions(-) create mode 100644 .agents/skills/test-audit/SKILL.md create mode 100644 LESSONS/20261011T225144Z-TEST-AUDIT-002-coordinator-isolate-negative-guards.md create mode 100644 docs/tasks/TEST-AUDIT-002.md diff --git a/.agents/skills/test-audit/SKILL.md b/.agents/skills/test-audit/SKILL.md new file mode 100644 index 0000000..a96efe9 --- /dev/null +++ b/.agents/skills/test-audit/SKILL.md @@ -0,0 +1,85 @@ +--- +name: test-audit +description: Audit test value when requested, and apply a behavior-first authoring gate to test changes. Preserve meaningful contracts, coverage, and the task's editing limits. +metadata: + version: "1.0" + status: released +--- + +# Test audit + +Use the installed delivery roles and owner-approved task scope. This supporting +skill adapts [OpenClaw's test audit](https://github.com/openclaw/openclaw/blob/e09f3ecd491db23f4fdc6e79b4349808d010f394/.agents/skills/test-audit/SKILL.md) +to dphtools. It adds neither permission nor another role. Use `implement-task` +for routine authors and `review-work` for independent reviewers. Specialist A/B +keep their blind inputs: they apply the authoring/value criteria without reading +implementation, source history, or other roles' conversations. + +## Authoring gate + +Before changing a test, identify its observable contract, a credible regression, +the gap in existing proof, and the real public or process boundary it exercises. +Extend an existing case table when it covers the same contract. Another test +layer needs a distinct risk, such as transport, lifecycle, or platform behavior. +Expected values must come from approved behavior or independent references and +invariants. Never add a production interface solely to make a test possible. + +A claimed bug regression needs evidence that valid input failed at the intended +behavior before repair. Existing-behavior and supplemental tests may pass on the +baseline; label them honestly rather than manufacturing red evidence. + +## Read-only audit + +For an authorized routine audit, inspect each complete candidate test, its real +owner/entry point, callers/callees, overlapping tests, routing, and relevant +history. Inspect applicable dependency source/types when the claim depends on +them. Parallelize independent discovery where useful, then select one coherent +owner-boundary batch. Broad campaigns need their own approved scope. + +Look for repeated proof, fixture self-comparisons, implementation-shaped greps, +private-helper replays, coverage-only execution, expectations calculated by the +subject, mocks/fixtures producing the promised result, negative cases rejected +by the wrong guard, and names overstating the exercised behavior. + +These patterns are evidence to investigate, not automatic deletion rules. +Retain independent public, platform, structural, packaging, release, and security +contracts; observable ordering; credible regressions; and hard-to-reach numerical +regimes. An externally used library API is not dead because only tests call it +in this repository. An approved exception or successful completion can be the +observable result. Static checks and slow tests can protect meaningful contracts. + +Before any deletion or consolidation, record: +- exact test location and the failure it can detect; +- non-test callers and the reason the test/support seam exists in history; +- stronger retained proof, or the approved reason no proof is needed; +- test/support edits enabled, risk, and focused validation command. + +Missing evidence leaves the candidate unchanged. A valid failing test remains +visible: classify it under specification section 4. A tests-only task reports a +product defect; it cannot repair production or delete the test to obtain green. + +## Repair, review, and completion + +Keep repairs within the owner's scope. Remove only demonstrated redundancy, +move retained regressions to their owning boundary, and simplify repeated test +setup without weakening checks or freezing implementation details. Production, +tooling, workflows, test discovery, and coverage policy remain untouched in a +tests-only audit. Instruction edits require their own owner authorization. + +Do not edit files while checks of that candidate are running. Run the focused +owner/sibling tests, targeted formatting, and `git diff --check`. Then classify +and verify the exact committed candidate with +`python tools/verification.py pr --base BASE`; retain the selected complete-domain +coverage requirements. A focused run is diagnostic, not full coverage evidence. + +Use a fresh independent reviewer after repairs. When the owner authorizes a +review/repair loop, return concrete findings to the author and renew review after +each changed candidate until acceptance or a real blocker. Respect inherited +budgets and specialist allowances; a new session does not reset them. Never let +the repair author approve their own changes. + +Report removed categories, retained false positives, focused/required evidence, +test versus production/tooling line changes, unresolved findings, and PR state. +Commit, push, PR creation, merge, and publication still need their applicable +authorization. Use dphtools' commands; OpenClaw's Node runners, linked campaign +and landing skills, and production-deletion preference are not installed here. diff --git a/AGENTS.md b/AGENTS.md index 425a445..db62c7e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,8 @@ Use one task per worktree. Routine work uses an author and one fresh independent - D uses `review-work` in candidate mode, forms findings before consulting current-task implementation lessons, and reviews actual behavior, evidence, and simplicity. Do not fix and approve your own repair. - Setup may create infrastructure when explicitly authorized. Doctor uses `review-work` in doctor mode and may edit documentation under `DOCTOR-PROMPT.md`; neither exception is permission for C to weaken checks. +Use the supporting [test-audit skill](.agents/skills/test-audit/SKILL.md) for requested test audits and its authoring criteria when changing tests. Keep its implementation-reading discovery on the routine route; blind A/B retain their restrictions. Audit findings do not authorize production edits. + Prefer real end-to-end/public-entry-point tests. Smaller tests fill genuine gaps through interfaces production code or real callers also need. Existing-code tests may pass initially; do not manufacture red evidence or require mutation testing. Require exact 100% measured statements and branches across every owned domain required by the owner-approved verification hierarchy whenever that domain runs, including never-imported files and relevant subprocesses. Canonical `full` remains fresh global and per-package 100% measurement. Narrow runs establish only their declared domains; do not represent an omitted domain or reused evidence as fresh coverage. Report unsupported measurement. Do not hide failures, exclusions, skips, or missing reports. Before repair, the observing role classifies failures using specification section 4: environment/tooling, test defect, product defect, or unresolved requirement. The coordinator routes work; classification grants no new editing scope. Diagnose uncertain causes before product repair. Only a valid test failing at the intended approved behavior supplies product-red evidence. Routine test/fixture changes need independent review; specialist test/fixture changes need fresh A/B and a revised checkpoint. Unchanged tests keep their checkpoint after environment repair. diff --git a/LESSONS/20261011T225144Z-TEST-AUDIT-002-coordinator-isolate-negative-guards.md b/LESSONS/20261011T225144Z-TEST-AUDIT-002-coordinator-isolate-negative-guards.md new file mode 100644 index 0000000..53bba00 --- /dev/null +++ b/LESSONS/20261011T225144Z-TEST-AUDIT-002-coordinator-isolate-negative-guards.md @@ -0,0 +1,9 @@ +# Isolate the guard a negative test claims to exercise + +- ID: 20261011T225144Z-TEST-AUDIT-002-coordinator-isolate-negative-guards +- Date: 2026-10-11T22:51:44Z +- Task/role: TEST-AUDIT-002 / coordinator +- Status: confirmed +- Observation: Invalid test inputs can trigger an earlier integrity or conservative-fallback guard, leaving the named semantic guard untested. +- Evidence: Base b995f584; test_verification_shards.py foreign run/platform cases retained stale seals; its index-duplicate case retained foreign executions; test_verification_scope.py invalid inventory also changed the inventory path, and unchanged-prefix proof added an unsupported call. Read-only owner tracing is retained in /private/tmp/dphtools-test-audit-002-boundaries.md. These are test-value findings, not mutation or product-red evidence. +- Lesson: Keep unrelated guards valid, isolate the candidate delta, and observe the intended rejection reason. Preserve separate byte-integrity/fallback controls. diff --git a/docs/agentic-software-delivery-v1.0/DELIVERY-SYSTEM-SPEC.md b/docs/agentic-software-delivery-v1.0/DELIVERY-SYSTEM-SPEC.md index 586c32a..43b8955 100644 --- a/docs/agentic-software-delivery-v1.0/DELIVERY-SYSTEM-SPEC.md +++ b/docs/agentic-software-delivery-v1.0/DELIVERY-SYSTEM-SPEC.md @@ -105,7 +105,7 @@ For each changed requirement, start with the smallest realistic success, failure Use integration or unit tests for a genuine gap: difficult error injection, combinatorial logic, or a branch that would make a whole-system test slow or brittle. No unit-test quota or fixed test-type percentage. Do not forbid valid negative assertions or property tests just because a heuristic dislikes them. -A behavioral test is implementation-coupled when a refactor preserving its approved observable contract would break the test. Test interfaces that production code or real callers also need; do not require production exports, wrappers, or injection hooks solely for tests. D investigates code referenced only by tests, while recognizing that a public library API may serve external callers. An explicit public, platform, or architectural contract can justify a structural check; suspicion alone is not authority to delete an existing test. These judgment criteria adapt [OpenClaw's test audit](REFERENCES.md#r10--openclaw-test-audit), not its implementation-reading workflow. +A behavioral test is implementation-coupled when a refactor preserving its approved observable contract would break the test. Test interfaces that production code or real callers also need; do not require production exports, wrappers, or injection hooks solely for tests. D investigates code referenced only by tests, while recognizing that a public library API may serve external callers. An explicit public, platform, or architectural contract can justify a structural check; suspicion alone is not authority to delete an existing test. These judgment criteria adapt [OpenClaw's test audit](REFERENCES.md#r10--openclaw-test-audit). The supporting [test-audit skill](../../.agents/skills/test-audit/SKILL.md) applies its authoring gate and evidence-first routine audit workflow under the installed roles. Routine audits may inspect implementation to establish test value; blind specialist A/B retain their source restrictions. An owner-authorized review/repair loop uses separate author and independent reviewer sessions, preserves inherited allowances, and ends at acceptance or a real blocker. Audit findings grant no product-editing scope. The independent reviewer (B on the specialist route) rejects execution solely to raise coverage without a meaningful correctness check; negative tests passing because an unrelated validation rejected the input; fixtures that supply the result whose production behavior is supposedly tested; and test names claiming more than their inputs exercise. A check need not use a literal assertion statement: an expected exception or successful completion may itself be approved observable behavior. Require the intended path and outcome, not a particular assertion syntax. diff --git a/docs/agentic-software-delivery-v1.0/REFERENCES.md b/docs/agentic-software-delivery-v1.0/REFERENCES.md index e07dea5..27f345c 100644 --- a/docs/agentic-software-delivery-v1.0/REFERENCES.md +++ b/docs/agentic-software-delivery-v1.0/REFERENCES.md @@ -72,6 +72,8 @@ Apply these as judgment criteria. A meaningful correctness check need not use a Do not import OpenClaw's implementation-reading audit, campaign, or landing workflows into blind A/B roles. Linked skills and commands were not read or installed. This source supplies no numerical-tolerance authority. The [inert archive](../references/openclaw/test-audit.source.txt) preserves the complete source with its [MIT notice](../references/licenses/openclaw-MIT.txt). +The owner subsequently authorized the supporting [test-audit skill](../../.agents/skills/test-audit/SKILL.md) in [TEST-AUDIT-002](../tasks/TEST-AUDIT-002.md). It adapts routine implementation-reading discovery and separate repair/review to dphtools, while preserving blind specialist inputs, editing scope, and verification. Upstream text was reread at `e09f3ecd491db23f4fdc6e79b4349808d010f394`; the original archive and setup provenance above remain historical. OpenClaw campaign, runner, review, and landing commands remain uninstalled. + ## Original articles and earlier supporting references The original four articles were revisited during setup. Their complete substantive prose was read, except that Symphony's embedded 1,363-line controller specification was deliberately excluded. The full narrative before and after that embed was read. OpenAI article bytes returned HTTP 403; the web tool supplied readable narrative text. No original-byte OpenAI article snapshot or article hash is claimed. Anthropic article bytes were retrieved and hashed, but full text is not republished without an identified redistribution license. [Reading notes](../references/READING-NOTES.md) distinguish the adopted ideas from excluded machinery. diff --git a/docs/tasks/TEST-AUDIT-002.md b/docs/tasks/TEST-AUDIT-002.md new file mode 100644 index 0000000..dd3b94c --- /dev/null +++ b/docs/tasks/TEST-AUDIT-002.md @@ -0,0 +1,91 @@ +# Test audit: verification + +## Authorization and candidate pointer + +The owner authorized adding the linked OpenClaw test-audit workflow to ASD, +running separate audit and repair agents until complete, and opening a PR. +Non-test code edits are forbidden. Instruction changes are the supporting skill +and ASD pointers; runtime, tooling, workflows, discovery, and coverage policy +remain outside repair scope. No merge or publication is authorized. + +Route: routine maintenance under the established local verification contract. +The two discovery lanes and separate author produced the test repairs below. +A fresh independent reviewer must inspect the candidate and applicable evidence. +No new scenario/behavior contract or specialist checkpoint is introduced. + +Base: `b995f5843edbb9247cdb3d9c0b1b08c97010e02a` on `codex-main`. +Branch: `codex/test-audit-verification`. This is the prepared task pointer: +the authoritative Current state, final candidate identity, commands/environment, +focused and required results, independent review, platform status, blockers, +next action, and launch/budget metrics are recorded in the conversation or +linked PR, outside this tracked candidate. This document preserves approved +scope and pre-edit evidence rather than a competing mutable status copy. + +The owner supplied no spending cap; the routine review/repair loop runs until +acceptance or a real blocker. Specialist allowances are not reset or used. +Skill frontmatter validation passed using a disposable YAML dependency without +changing the locked verification environment. A confirmed metrics product defect +is explicitly deferred below because runtime code repair is forbidden. + +## Scope and method + +Apply [.agents/skills/test-audit](../../.agents/skills/test-audit/SKILL.md). +Upstream text was read at `e09f3ecd491db23f4fdc6e79b4349808d010f394` +(SHA-256 `fc38d2a34ade41960355d951b018dea1a0dfc89faca28914ba2c9a45d007fa3f`). +The original inert archive and R10 setup provenance remain unchanged. This is +a local adaptation; OpenClaw campaign/Node/landing skills are not installed. + +The coherent batch is `tests/test_verification*.py` and directly used test +support, including [TEST-AUDIT-001's named follow-ups](TEST-AUDIT-001.md#named-follow-ups-not-edits-in-this-batch). +Auditors inspected complete tests, runtime owners/entry points/callers/callees, +overlaps, CI routing, applicable installed dependency sources/types, and history. +Source reading was confined to routine discovery; blind A/B restrictions remain. + +Full pre-edit candidate fields and retained false positives are recorded outside +Git in [core evidence](/private/tmp/dphtools-test-audit-002-core.md) and +[boundary evidence](/private/tmp/dphtools-test-audit-002-boundaries.md). +Locations below refer to the base, not shifted candidate lines. + +| Finding | Evidence and authorized test repair | Retained proof | +|---|---|---| +| Fixture coverage rereads | `test_verification.py:515` checks facts authored by its external-tool fixture. Remove redundant rereads. | Real coverage instrumentation, never-imported files and all invalid-report cases. | +| Duplicate lint launches | `test_verification.py:908` directly reruns the same dependency already reached by the real fast command. Remove direct probes. | Public fast success and precise F821/file rejection. | +| Incidental total order | `test_verification.py:503,702,781,1200` fixes independent operations to one sequence. Reuse inventory/dependency observations. | No missing/duplicate operations; quality/build/install/test/report prerequisites; receipts matching actual calls; live streaming. | +| Vacuous identity assertion | `test_verification.py:1392` OR is already forced true by changed tags. Remove it and its unused local. | Actual Git/tag/history/dirty/version/artifact observations and changed-input reuse proof. | +| Foreign identity masked by seal | `test_verification_shards.py:283` changes run/platform but retains a stale digest. Reseal to reach semantic identity rejection. | Separate byte-tamper cases and explicit intended-guard diagnostics. | +| Duplicate-index case masks its claim | `test_verification_shards.py:530` reaches foreign execution first. Remove the redundant parameter/branch. | Real CLI duplicate-directory rejection and remaining semantic/execution cases. | +| Invalid inventory masked by path fallback | `test_verification_scope.py:200` changes the inventory file itself. Establish invalid inventory in fixture base, then change only prose and require disclosed inventory failure. | Valid-inventory prose control and all closure/fallback cases. | +| Dynamic prefix masked by new call | `test_verification_scope.py:293` introduces an unsupported call. Change only a literal while retaining unchanged helper/hook prefixes. | Existing ordinary literal control and unsupported-call tests. | +| Dead support | Unused hook full observer branch, shard resealer and scope imports. Reuse support where meaningful; remove the remainder. | Every hook process/platform/backup test and complete receipt/measurement cases. | + +Real installation, receipt reuse, Versioneer, lock/startup bytes, hook platform +behavior, sharded parent/child coverage, lifecycle, report integrity, and metrics +parser tests stay. A suspicious pattern alone does not justify deletion. +No runtime seam is removed; no threshold, exclusion, skip, or discovery rule changes. + +## Deferred product issue + +At the base, `tools/verification_metrics.py:88` accepts old receipt modes but +rejects `library`, `doctor`, and `release`. Minimal temporary orchestration +fixtures invoked the unchanged public verifier for each mode. Each returned 0 +with a complete passed version 2.0 receipt and the requested actual mode. +The unchanged metrics CLI rejected each with exit 1 and +`metrics: Unknown receipt mode`. + +[Probe results](/private/tmp/dphtools-test-audit-002-metrics-probe/results.json) +retain exact receipts and logs. These controlled external-tool fixtures confirm +CLI/receipt compatibility only; they are not real product coverage or a successful +domain/full gate. No branch test is removed or introduced to conceal this issue. +Repairing that runtime code requires a separately authorized task. + +## Validation and release impact + +Run focused modified-owner/sibling tests, targeted Black and `git diff --check`, +then `python tools/verification.py pr --base origin/codex-main` on the exact +committed candidate. Added instruction/task files and verification changes +require Tier 5: fresh full verification, exact 100% statements/branches globally +and per package. Hosted Linux/macOS/Windows evidence stays pending until observed. + +No package version, public behavior, or publication change. Draft release note: +verification tests retain stronger process proof, isolate intended rejection +guards, and ASD gains a supporting test-audit workflow. diff --git a/tests/test_verification.py b/tests/test_verification.py index 84f54c0..4a2f637 100644 --- a/tests/test_verification.py +++ b/tests/test_verification.py @@ -4,6 +4,7 @@ controlled subprocesses; their reports are fixtures, not numerical coverage proof. """ +from collections import Counter from io import BytesIO from importlib.metadata import distribution import codecs @@ -484,15 +485,16 @@ def test_fast_runs_all_tools_from_script_repository(verifier): result = verifier.run("fast") assert result.returncode == 0, _detail(result) calls = verifier.calls() - assert [call["tool"] for call in calls] == ["black", "flake8", "pydocstyle"] + _assert_check_operations(calls, FAST_OPERATIONS) assert all(Path(call["cwd"]).resolve() == verifier.repo.resolve() for call in calls) assert all(Path(call["python"]) == Path(sys.executable) for call in calls) - black_args = calls[0]["args"] + black = next(call for call in calls if call["tool"] == "black") + black_args = black["args"] assert "--check" in black_args assert "--line-length=99" in black_args or any( pair == ["--line-length", "99"] for pair in [black_args[index : index + 2] for index in range(len(black_args))] - ), calls[0] + ), black _check_manifest(verifier, result, "fast") @@ -500,7 +502,7 @@ def test_full_success_orders_tools_and_retains_reports(verifier): result = verifier.run("full") assert result.returncode == 0, _detail(result) calls = verifier.calls() - assert _sequence(calls) == FULL_SEQUENCE + _assert_check_operations(calls, FULL_OPERATIONS) assert all(Path(call["python"]) == Path(sys.executable) for call in calls) assert all(Path(call["cwd"]).resolve() == verifier.repo.resolve() for call in calls) audit = next(call for call in calls if call["tool"] == "pip_audit") @@ -512,11 +514,6 @@ def test_full_success_orders_tools_and_retains_reports(verifier): assert {"pytest.xml", "coverage.json", "coverage.xml"} <= { path.name for path in report.iterdir() } - coverage = json.loads((report / "coverage.json").read_text(encoding="utf-8")) - assert "dphtools/never_imported.py" in coverage["files"] - assert "tools/verification.py" in coverage["files"] - assert "dphtools/_version.py" not in coverage["files"] - assert coverage["totals"]["num_branches"] == 0 _assert_full_operations(verifier, calls) @@ -621,7 +618,7 @@ def commit(): assert checks["scope"]["name"] == "fast" and checks["coverage_complete"] is False assert checks["scope"]["classification"]["base_sha"] == base assert checks["scope"]["classification"]["head_sha"] == head - assert _sequence(verifier.calls()) == ["black", "flake8", "pydocstyle"] + _assert_check_operations(verifier.calls(), FAST_OPERATIONS) def _assert_full_operations(verifier, calls): @@ -659,7 +656,7 @@ def test_fast_attempts_later_tools_after_failure(verifier, failed): verifier.env["VERIFICATION_TEST_FAIL"] = failed result = verifier.run("fast") assert result.returncode == 1, _detail(result) - assert [call["tool"] for call in verifier.calls()] == ["black", "flake8", "pydocstyle"] + _assert_check_operations(verifier.calls(), FAST_OPERATIONS) _check_manifest(verifier, result, "fast", failed) @@ -699,16 +696,13 @@ def test_full_rejects_invalid_reports_even_when_all_tools_pass(verifier, scenari verifier.env["VERIFICATION_TEST_REPORT"] = scenario result = verifier.run("full") assert result.returncode == 1, _detail(result) - assert _sequence(verifier.calls()) == FULL_SEQUENCE, _detail(result) + _assert_check_operations(verifier.calls(), FULL_OPERATIONS) _check_manifest(verifier, result, "full") -FULL_SEQUENCE = [ - "black", - "flake8", - "pydocstyle", - "mypy", - "pip_audit", +FAST_OPERATIONS = ["black", "flake8", "pydocstyle"] +QUALITY_OPERATIONS = FAST_OPERATIONS + ["mypy", "pip_audit"] +FULL_OPERATIONS = QUALITY_OPERATIONS + [ "build", "pip", "coverage:erase", @@ -727,6 +721,25 @@ def _sequence(calls): ] +def _assert_check_operations(calls, expected): + """Observe operation counts and required lifecycle edges, including partial runs.""" + observed = _sequence(calls) + assert Counter(observed) == Counter(expected), observed + dependencies = [(quality, "build") for quality in QUALITY_OPERATIONS] + [ + ("build", "pip"), + ("pip", "coverage:erase"), + ("pip", "coverage:run"), + ("coverage:erase", "coverage:run"), + ("coverage:run", "coverage:combine"), + ("coverage:combine", "coverage:json"), + ("coverage:combine", "coverage:xml"), + ("coverage:combine", "coverage:report"), + ] + for before, after in dependencies: + if before in expected and after in expected: + assert observed.index(before) < observed.index(after), (before, after, observed) + + def _check_receipt_seal(data): """The public receipt seal binds the complete current record, including timing.""" unsigned = {key: value for key, value in data.items() if key != "receipt_digest"} @@ -778,30 +791,36 @@ def _check_manifest(verifier, result, mode, failed=None): return report -@pytest.mark.parametrize("failed", FULL_SEQUENCE) +@pytest.mark.parametrize("failed", FULL_OPERATIONS) def test_full_failure_blocks_dependents_and_retains_fresh_diagnostics(verifier, failed): """The former all-later-steps contract is replaced by explicit prerequisites.""" verifier.env["VERIFICATION_TEST_FAIL"] = failed result = verifier.run("full") assert result.returncode == 1, _detail(result) - if failed in FULL_SEQUENCE[:5]: - expected = FULL_SEQUENCE[:5] + if failed in QUALITY_OPERATIONS: + expected = QUALITY_OPERATIONS elif failed == "build": - expected = FULL_SEQUENCE[:6] + expected = QUALITY_OPERATIONS + ["build"] elif failed == "pip": - expected = FULL_SEQUENCE[:7] + expected = QUALITY_OPERATIONS + ["build", "pip"] elif failed == "coverage:erase": - expected = FULL_SEQUENCE[:8] + expected = QUALITY_OPERATIONS + ["build", "pip", "coverage:erase"] elif failed == "coverage:combine": - expected = FULL_SEQUENCE[:10] + expected = QUALITY_OPERATIONS + [ + "build", + "pip", + "coverage:erase", + "coverage:run", + "coverage:combine", + ] else: - expected = FULL_SEQUENCE - assert _sequence(verifier.calls()) == expected + expected = FULL_OPERATIONS + _assert_check_operations(verifier.calls(), expected) report = _check_manifest(verifier, result, "full", failed) data = json.loads((report / "checks.json").read_text(encoding="utf-8")) assert all(step["state"] in {"passed", "failed", "blocked"} for step in data["steps"]) assert all(step["duration_seconds"] >= 0 for step in data["steps"]) - if len(expected) < len(FULL_SEQUENCE): + if len(expected) < len(FULL_OPERATIONS): assert any( step["state"] == "blocked" and step["blocking_reasons"] for step in data["steps"] ) @@ -905,33 +924,16 @@ def test_real_lint_configuration_retains_critical_errors(verifier): for name in ("setup.py", "versioneer.py"): (verifier.repo / name).write_text('"""Tooling fixture."""\n', encoding="utf-8") - def lint_fixture(): - return subprocess.run( - [sys.executable, "-m", "flake8", "dphtools"], - cwd=verifier.repo, - env=verifier.env, - capture_output=True, - text=True, - encoding="utf-8", - timeout=20, - check=False, - ) - - style_only = lint_fixture() - assert style_only.returncode == 0, _detail(style_only) public_style = verifier.run("fast") assert public_style.returncode == 0, _detail(public_style) - assert _sequence(verifier.calls()) == ["black", "pydocstyle"] + _assert_check_operations(verifier.calls(), ["black", "pydocstyle"]) verifier.record.unlink() target.write_text("VALUE = undefined_fixture_name\n", encoding="utf-8") - critical = lint_fixture() - assert critical.returncode != 0, _detail(critical) - assert "F821" in critical.stdout + critical.stderr, _detail(critical) public_critical = verifier.run("fast") assert public_critical.returncode == 1, _detail(public_critical) assert "deep.py" in public_critical.stdout + public_critical.stderr, _detail(public_critical) assert "F821" in public_critical.stdout + public_critical.stderr, _detail(public_critical) - assert _sequence(verifier.calls()) == ["black", "pydocstyle"] + _assert_check_operations(verifier.calls(), ["black", "pydocstyle"]) def test_undecodable_child_output_retains_failure_diagnostics_and_later_checks(verifier): @@ -978,7 +980,7 @@ def test_undecodable_child_output_retains_failure_diagnostics_and_later_checks(v result = verifier.run("fast") assert result.returncode == 1, _detail(result) - assert _sequence(verifier.calls()) == ["black", "flake8", "pydocstyle"], _detail(result) + _assert_check_operations(verifier.calls(), FAST_OPERATIONS) report = _check_manifest(verifier, result, "fast", failed="black") logs = "\n".join(path.read_text(encoding="utf-8") for path in report.glob("*.log")) for destination in (result.stdout + result.stderr, logs): @@ -1124,14 +1126,12 @@ def deny_black_launch(event, args): assert receipt.is_file(), f"OS launch-fault fixture was not exercised: {_detail(result)}" denied = json.loads(receipt.read_text(encoding="utf-8"))["command"] assert result.returncode == 1, _detail(result) - assert _sequence(verifier.calls()) == ["flake8", "pydocstyle"], _detail(result) + _assert_check_operations(verifier.calls(), ["flake8", "pydocstyle"]) assert all(Path(call["python"]) == Path(sys.executable) for call in verifier.calls()) reports = verifier.reports() assert len(reports) == 1, _detail(result) data = json.loads(reports[0].read_text(encoding="utf-8")) assert data["document_version"] == "2.0" and data["mode"] == "fast" - calls = verifier.calls() - calls = verifier.calls() steps = [ step for step in data["steps"] @@ -1143,12 +1143,14 @@ def deny_black_launch(event, args): and step["command"][2] in {"black", "flake8", "pydocstyle"} ] assert len(steps) == 3 - reported_command = steps[0]["command"] + by_tool = {step["command"][2]: step for step in steps} + reported_command = by_tool["black"]["command"] if isinstance(denied, str): reported_command = subprocess.list2cmdline(reported_command) assert reported_command == denied - assert isinstance(steps[0]["returncode"], int) and steps[0]["returncode"] != 0 - for step, call in zip(steps[1:], verifier.calls()): + assert isinstance(by_tool["black"]["returncode"], int) and by_tool["black"]["returncode"] != 0 + for call in verifier.calls(): + step = by_tool[call["tool"]] assert step["command"] == [sys.executable, "-m", call["tool"], *call["args"]] assert step["returncode"] == 0 diagnostic = "fixture scheduled launch unavailable" @@ -1201,7 +1203,7 @@ def test_full_rejects_broken_preflight_before_build(verifier): (verifier.repo / "requirements-dev.lock").write_text("coverage==0.0\n", encoding="utf-8") result = verifier.run("full") assert result.returncode == 1, _detail(result) - assert _sequence(verifier.calls()) == FULL_SEQUENCE[:5] + _assert_check_operations(verifier.calls(), QUALITY_OPERATIONS) data = json.loads(verifier.reports()[0].read_text(encoding="utf-8")) assert next(step for step in data["steps"] if step["name"] == "build")["state"] == "blocked" @@ -1211,7 +1213,7 @@ def test_full_requires_exactly_one_built_wheel(verifier, scenario): verifier.env["VERIFICATION_TEST_REPORT"] = scenario result = verifier.run("full") assert result.returncode == 1, _detail(result) - assert _sequence(verifier.calls()) == FULL_SEQUENCE[:6] + _assert_check_operations(verifier.calls(), QUALITY_OPERATIONS + ["build"]) data = json.loads(verifier.reports()[0].read_text(encoding="utf-8")) assert ( next(step for step in data["steps"] if step["name"] == "wheel-artifacts")["state"] @@ -1226,7 +1228,9 @@ def test_failed_tests_without_fresh_data_block_coverage_diagnostics(verifier): ) result = verifier.run("full") assert result.returncode == 1, _detail(result) - assert _sequence(verifier.calls()) == FULL_SEQUENCE[:9] + _assert_check_operations( + verifier.calls(), QUALITY_OPERATIONS + ["build", "pip", "coverage:erase", "coverage:run"] + ) data = json.loads(verifier.reports()[0].read_text(encoding="utf-8")) assert ( next(step for step in data["steps"] if step["name"] == "coverage-json")["state"] @@ -1362,7 +1366,6 @@ def test_build_identity_tracks_actual_git_tags_history_dirty_state_and_version(v record = json.loads(verifier.reports()[0].read_text(encoding="utf-8")) build = next(step for step in record["steps"] if step["name"] == "build") original = build["input_identity"]["git"] - original_inputs = record["identity"]["inputs"] assert original["revision"]["returncode"] == 0 assert "1.0.0" in original["tags"]["output"] assert original["history"]["output"] == original["revision"]["output"] @@ -1389,7 +1392,6 @@ def test_build_identity_tracks_actual_git_tags_history_dirty_state_and_version(v assert updated["revision"] == original["revision"] assert updated["tags"] != original["tags"] assert "never_imported.py" in updated["dirty"]["output"] - assert build["input_identity"]["inputs"] != record["identity"]["inputs"] or updated != original install = next(step for step in record["steps"] if step["name"] == "install") assert install["input_identity"]["artifacts"] diff --git a/tests/test_verification_hooks.py b/tests/test_verification_hooks.py index a853f90..bf12149 100644 --- a/tests/test_verification_hooks.py +++ b/tests/test_verification_hooks.py @@ -1,6 +1,5 @@ """Real Git hook entry points in isolated repositories, with local-only remotes.""" -from collections import Counter import json import os from pathlib import Path @@ -9,8 +8,8 @@ import pytest -from .test_verification import FULL_SEQUENCE, ROOT, VerificationCommand, _detail, _executable -from .test_verification import _sequence +from .test_verification import ROOT, VerificationCommand, _detail, _executable +from .test_verification import FAST_OPERATIONS, QUALITY_OPERATIONS, _assert_check_operations from .test_verification import TOOL from .test_verification import _prove_executable_runtime from .test_verification import _check_manifest @@ -158,7 +157,7 @@ def test_pre_commit_checks_staged_content_with_selected_interpreter(hook_repo, s before = hook_repo.git("write-tree").stdout result = hook_repo.hook() assert result.returncode == 0, _detail(result) - assert _sequence(hook_repo.calls()) == ["black", "flake8", "pydocstyle"] + _assert_check_operations(hook_repo.calls(), FAST_OPERATIONS) calls = hook_repo.python_calls() assert len(calls) == 1 and calls[0]["argv"][-1] == "fast", calls expected = hook_repo.selected_python if selection == "explicit" else hook_repo.default_python @@ -186,7 +185,7 @@ def test_pre_commit_rejects_failed_fast_checks(hook_repo): hook_repo.env["VERIFICATION_TEST_FAIL"] = "black" result = hook_repo.hook() assert result.returncode != 0, _detail(result) - assert _sequence(hook_repo.calls()) == ["black", "flake8", "pydocstyle"] + _assert_check_operations(hook_repo.calls(), FAST_OPERATIONS) @pytest.mark.parametrize("selection", ["default", "explicit"]) @@ -211,27 +210,8 @@ def _assert_clean_head(hook_repo): assert hook_repo.git("status", "--porcelain", "--untracked-files=all").stdout == "" -def _assert_check_operations(calls, mode): - observed = _sequence(calls) - expected = FULL_SEQUENCE if mode == "full" else ["black", "flake8", "pydocstyle"] - # Reuse the operation inventory, not its incidental total ordering. - assert Counter(observed) == Counter(expected), observed - if mode == "full": - dependencies = [ - ("build", "pip"), - ("pip", "coverage:run"), - ("coverage:erase", "coverage:run"), - ("coverage:run", "coverage:combine"), - ("coverage:combine", "coverage:json"), - ("coverage:combine", "coverage:xml"), - ("coverage:combine", "coverage:report"), - ] - for before, after in dependencies: - assert observed.index(before) < observed.index(after), (before, after, observed) - - def _assert_fast_invocation(hook_repo, selection="default"): - _assert_check_operations(hook_repo.calls(), "fast") + _assert_check_operations(hook_repo.calls(), FAST_OPERATIONS) calls = hook_repo.python_calls() assert len(calls) == 1 and len(calls[0]["argv"]) == 2, calls assert calls[0]["argv"][1] == "fast", calls @@ -312,7 +292,7 @@ def test_pre_push_full_only_failure_allows_real_local_backup(hook_repo): assert full.returncode == 1, _detail(full) # The former all-later-tools expectation is replaced by the authorized DAG: # audit failure preserves independent cheap results and blocks costly dependents. - assert _sequence(hook_repo.calls()) == ["black", "flake8", "pydocstyle", "mypy", "pip_audit"] + _assert_check_operations(hook_repo.calls(), QUALITY_OPERATIONS) full_report = _check_manifest(hook_repo, full, "full", "pip_audit") receipt = json.loads((full_report / "checks.json").read_text(encoding="utf-8")) assert receipt["complete"] is True and receipt["outcome"] == "failed" @@ -441,16 +421,16 @@ def _arrange_state_change(hook_repo, mutation): return receipt, status -def _assert_successful_check(hook_repo, mode, status): +def _assert_successful_check(hook_repo, status): assert status.is_file(), "The real verifier did not finish through the interpreter fixture" assert status.read_text(encoding="utf-8") == "0", "Verifier failed before stale-state check" calls = hook_repo.python_calls() - assert len(calls) == 1 and calls[0]["argv"][-1] == mode, calls - _assert_check_operations(hook_repo.calls(), mode) + assert len(calls) == 1 and calls[0]["argv"][-1] == "fast", calls + _assert_check_operations(hook_repo.calls(), FAST_OPERATIONS) reports = hook_repo.reports() assert len(reports) == 1 data = json.loads(reports[0].read_text(encoding="utf-8")) - assert data["mode"] == mode + assert data["mode"] == "fast" assert data["steps"] and all(step["returncode"] == 0 for step in data["steps"]) @@ -462,7 +442,7 @@ def test_pre_commit_rejects_content_changed_during_successful_checks(hook_repo, receipt, status = _arrange_state_change(hook_repo, mutation) result = hook_repo.hook() - _assert_successful_check(hook_repo, "fast", status) + _assert_successful_check(hook_repo, status) change = json.loads(receipt.read_text(encoding="utf-8")) assert change["before"] == {"head": hook_repo.head, "index": index_before} assert change["after"]["head"] == hook_repo.head @@ -485,7 +465,7 @@ def test_pre_push_rejects_state_changed_during_successful_checks(hook_repo, muta receipt, status = _arrange_state_change(hook_repo, mutation) result = hook_repo.push("HEAD:refs/heads/main") - _assert_successful_check(hook_repo, "fast", status) + _assert_successful_check(hook_repo, status) _assert_fast_invocation(hook_repo) change = json.loads(receipt.read_text(encoding="utf-8")) assert change["before"] == {"head": hook_repo.head, "index": index_before} @@ -515,7 +495,7 @@ def test_pre_push_rejects_index_changed_during_successful_checks(hook_repo): receipt, status = _arrange_state_change(hook_repo, "staged") result = hook_repo.push("HEAD:refs/heads/main") - _assert_successful_check(hook_repo, "fast", status) + _assert_successful_check(hook_repo, status) _assert_fast_invocation(hook_repo) change = json.loads(receipt.read_text(encoding="utf-8")) assert change["before"] == {"head": hook_repo.head, "index": index_before} diff --git a/tests/test_verification_scope.py b/tests/test_verification_scope.py index 87ae64d..927a844 100644 --- a/tests/test_verification_scope.py +++ b/tests/test_verification_scope.py @@ -1,9 +1,7 @@ """Approved trigger hierarchy through real Git and verifier entry points.""" import json -import os from pathlib import Path -import shutil import subprocess import sys @@ -206,8 +204,17 @@ def test_invalid_domain_inventory_falls_back_and_is_visible(repository, defect): repository.save_mapping() else: repository.write("tools/verification-domains.json", "invalid JSON") + repository.base = repository.commit() + repository.write("docs/notes.md", "Changed plain notes.\n") repository.commit() - assert repository.plan()["name"] == "full" + result = repository.plan() + assert result["name"] == "full" + expected = ( + "Expecting value" + if defect == "invalid-json" + else "Test ownership is duplicate or names absent files" + ) + assert any(expected in reason for reason in result["classification"]["reasons"]), result def test_explicit_scope_includes_nested_never_imported_sources_and_unknown_fallback(repository): @@ -293,7 +300,7 @@ def test_body_identical_comment_edits_do_not_invent_changed_effects(repository): def test_unchanged_dynamic_helpers_or_import_hooks_require_full(repository, prefix): repository.write("dphtools/core.py", prefix + "def value():\n return 42\n") repository.base = repository.commit() - repository.write("dphtools/core.py", prefix + "def value():\n return helper()\n") + repository.write("dphtools/core.py", prefix + "def value():\n return 43\n") repository.commit() assert repository.plan()["name"] == "full" diff --git a/tests/test_verification_shards.py b/tests/test_verification_shards.py index 173715b..09519e3 100644 --- a/tests/test_verification_shards.py +++ b/tests/test_verification_shards.py @@ -298,15 +298,26 @@ def test_aggregate_rejects_incomplete_foreign_and_changed_receipts( with artifact.open("ab") as stream: stream.write(b"tampered") else: - receipt = json.loads((shards[0] / "shard.json").read_text()) - receipt["identity"]["run"]["id"] = "old" if fault == "run" else "fixture-run" - if fault == "platform": - receipt["identity"]["environment"]["system"] = "Foreign" - (shards[0] / "shard.json").write_text(json.dumps(receipt)) + + def foreign_identity(receipt): + if fault == "run": + receipt["identity"]["run"]["id"] = "old" + else: + receipt["identity"]["environment"]["system"] = "Foreign" + + _reseal(shards[0] / "shard.json", foreign_identity) result = command.aggregate(tmp_path / "aggregate", shards) assert result.returncode == 1, result.stdout + result.stderr receipt = json.loads((tmp_path / "aggregate/checks.json").read_text()) assert receipt["outcome"] == "failed" + if fault in ("run", "platform", "duplicate"): + expected = ( + "Duplicate or invalid shard index" + if fault == "duplicate" + else "Shard inputs/platform/manifest/current run differ" + ) + diagnostic = (tmp_path / "aggregate/coverage-data.log").read_text() + assert expected in diagnostic, diagnostic assert not (tmp_path / "aggregate/coverage.json").exists() @@ -395,7 +406,6 @@ def _reseal(path, mutate): "foreign-manifest", "index-type", "index-range", - "index-duplicate", "count", "nodes", "missing-step", @@ -528,9 +538,7 @@ def test_production_aggregate_rejects_semantic_receipt_faults( elif fault == "foreign-manifest": receipt["manifest"] = "foreign" elif fault.startswith("index-"): - receipt["index"] = {"index-type": "0", "index-range": 2, "index-duplicate": 1}[fault] - if fault == "index-duplicate": - receipt["nodes"] = json.loads(command.manifest.read_text())["assignments"][1] + receipt["index"] = {"index-type": "0", "index-range": 2}[fault] elif fault == "count": receipt["count"] = 3 elif fault == "nodes":