Skip to content

fix(ci): fail vector-result merge on vacuous or inconsistent artifacts#58

Open
yilirong1992 wants to merge 2 commits into
tempoxyz:mainfrom
yilirong1992:fix/merge-results-empty-artifacts
Open

fix(ci): fail vector-result merge on vacuous or inconsistent artifacts#58
yilirong1992 wants to merge 2 commits into
tempoxyz:mainfrom
yilirong1992:fix/merge-results-empty-artifacts

Conversation

@yilirong1992

Copy link
Copy Markdown

merge_vector_results: reject vacuous or self-inconsistent result artifacts

Summary

The vector-result merger trusted every readable artifact. That leaves a false-PASS
hole in CI: a results-<adapter>.json that parses as JSON but carries no actual
conformance data merges into an overall status=pass.

Reproducible on main before this change:

$ echo '{}' > vector-results/results-go.json
$ python3 scripts/merge_vector_results.py --adapters go --result-dir vector-results --output merged.json
Vector status: pass        # 0 checks, 0 failures — green

Concrete ways to hit this:

  • the runner is killed after the artifact file is created but before checks run
    (truncated-but-valid JSON, or an empty object),
  • an upload/download step swaps in a wrong or stale file,
  • a future runner regression emits status: "fail" with malformed counts —
    the merger ignored the status field entirely and only summed failed.

Since branch protection keys on the merged status, any of these silently green-lights
a PR that never ran conformance for that adapter.

What this PR does

Each artifact must now carry real evidence of a run:

  1. Non-object payloads ([], a bare string, …) are treated like unreadable
    files and recorded as a runner failure for that adapter.
  2. Zero-check artifacts (num_checks == 0, including {}) fail with
    "... reported zero conformance checks". The vector runner always emits at
    least one check (it has an explicit no_checks guard), so a zero-check
    artifact can only mean the run did not happen.
  3. Status/count inconsistency (status != "pass" but failed == 0) fails
    instead of being ignored — the merger now honors the artifact's own status
    field as a failure signal.
  4. An empty --adapters list produces a failing merge instead of a green
    run with zero checks.

Counting and error propagation for healthy artifacts is unchanged, as is the
$GITHUB_OUTPUT interface consumed by conformance.yml and the monitor workflow.

Tests

New test_merge_vector_results.py (picked up by the existing
unittest discover step in ci.yml) covers: healthy merge, {} artifact,
zero-check artifact, status:"fail" with zero counts, non-object payload,
missing artifact file, and the empty adapter list. Full script suite:
16 tests, OK.

A results-<adapter>.json containing {} (or any object with zero checks)
merged into an overall pass, and the artifact's own status field was
ignored, so a run that never executed could green-light CI. Require
every artifact to contribute checks, honor status/count consistency,
and fail on an empty adapter list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant