Repository navigation
ci: avoid counters already excluded from coverage reports - #470
Conversation
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep review of 1a19690 against d725189: no blocking findings.
The existing report exclusion predicate remains the single policy owner. The new mode narrows only -coverpkg; the unchanged test_scope still reaches go test. Verified against matrixone#31237's actual package inventory: 505 test packages, 484 instrumented packages, excluding only 20 pkg/pb packages plus goyacc. Excluded-package tests still execute. Helper checkout is pinned to job.workflow_sha, and the same compiled helper is reused for selection and compaction. Build/selection errors stop the phase; test supervision and compaction failure handling remain intact.
Independent validation:
- All seven helper tests pass locally under -race -count=1 (4.818s); go vet, formatting and diff checks pass.
- The real four-package Go fixture proves excluded-package tests and their hits in retained dependencies still execute, untested zero-hit blocks remain in the denominator, and compacted old/new profiles are byte-identical. Empty/invalid scope and read/write failures are covered.
- Exact-head CI job 121429185175 passed; its log confirms actionlint plus helper tests/vet executed.
Complexity is proportionate: helper net +36, workflow net +8, tests +94, no new documentation/evidence files. The added test uses a small real Go module rather than duplicating coverage behavior in a mock.
Performance boundary: this removes redundant counters/profile records. The PR's measurements do not establish a reduction in peak memory; the recorded memory peaks increased in those samples. No claim of full-job memory improvement or complete 505-package combined performance validation is made. MatrixOne's currently running workflow uses CI@main, so it is not an end-to-end run of this unmerged CI revision. The changes have no hard merge-order dependency.
No existing review threads were present at recheck. Submitting COMMENT because the authenticated account is also the PR author.
What type of PR is this?
Which issue(s) this PR fixes:
Related to matrixorigin/matrixone#29562 and matrixorigin/matrixone#31146.
What this PR does / why we need it:
The coverage report already discards generated protobuf/goyacc records after collecting them. Apply the same policy before instrumentation, keeping the complete test-package selection. In the measured main inventory this runs all 505 test packages and instruments 484. Tests inside excluded packages still contribute hits to retained dependencies.
Reuse the existing compactor as the sole exclusion-policy owner. Compile it once during bounded preparation and reuse that binary for final compaction. Empty/invalid scope, build failure and profile errors remain fatal. Keep package parallelism 8, cluster admission pool 2, the 20-minute package timeout and existing producer supervision.
Validation: