Do not fail coverage jobs when the profdata artifact is missing - #109250
Conversation
LLVM coverage jobs (Integration, Functional and Unit tests, amd_llvm_coverage)
run their coverage merge non-blocking: llvm-profdata can crash on a corrupt
.profraw and produce no .profdata, but the tests all pass and the job is meant
to stay green ("do not block pipeline").
Each such job also declares provides=[LLVM_COVERAGE_FILE_*] with path
./*.profdata. The praktika post-run upload treated a missing providing artifact
as a hard error and set the job to check_status=error even though 0 tests
failed. Observed on several PRs where job.log shows "Failed to create final
coverage file" (llvm-profdata crash) followed by "Failed to upload artifact
[LLVM_COVERAGE_FILE_it_N:./*.profdata]".
Add an optional flag to Artifact.Config. When a providing artifact marked
optional matches no file at upload time, skip it with a warning instead of
reddening the job. Mark the coverage profdata artifacts optional. The
downstream LLVM Coverage aggregation already globs whatever .profdata files
exist, so a missing batch is tolerated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pre-PR validation gate (click to expand)
Additional context: the job-level-error-with-all-tests-OK signature was cited on 4 PRs on 2026-07-02 (batches 5/8 + 6/8), 0 on master. Example failing report: PR #109097, Integration tests (amd_llvm_coverage, 6/8) - its Session id: cron:clickhouse-worker-slot-1:20260703-021200 |
|
cc @maxknv @leshikus - could you review this praktika change? When an LLVM coverage job's |
PedroTadim
added
the
can be tested
|
Workflow [PR], commit [cf8cb51] Summary: ✅ AI ReviewSummaryThis PR adds Findings❌ Blockers
Tests
Final Verdict
|
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
| # tolerated by the downstream LLVM Coverage aggregation, which globs | ||
| # whatever .profdata files exist, so a missing file must not redden a | ||
| # coverage job whose tests all passed. | ||
| optional=True, |
There was a problem hiding this comment.
Marking every LLVM_COVERAGE_FILE_* artifact optional here applies to the master workflow too, not just PR coverage jobs. That breaks an invariant we already rely on in ci/jobs/scripts/workflow_hooks/filter_job.py: lines 342-345 explicitly say master must always publish a complete llvm_coverage.info, otherwise later PRs get artificial coverage deltas from a partial baseline. With this change a shard that fails its local llvm-profdata merge will stay green, Runner._post_run will silently skip the missing artifact, and the final merge still proceeds over whatever *.profdata happen to exist (ci/jobs/scripts/merge_llvm_coverage.sh, lines 52-72). The result is that master can now upload an incomplete baseline instead of failing noisy.
Can we keep the non-blocking behavior for PR jobs without weakening master? Two options that would preserve the coverage contract are:
- make the artifact optional only on PR workflows, or
- keep the producer green but have the final
LLVM Coveragejob detect missing expected shards and refuse to publishllvm_coverage.infoonmaster.
There was a problem hiding this comment.
Confirmed and fixed in 814773d. You are right: the blanket optional=True in defs.py is shared by the master workflow too, so a master shard whose non-blocking llvm-profdata merge crashed would have stayed green and published a partial baseline, giving later PRs artificial deltas.
Went with option 1 (PR-only). The artifact config in defs.py is static and shared by both workflows, so it cannot distinguish PR from master; instead the gate is at the runtime skip decision, where the event is known. Runner._skip_missing_optional_artifact now takes env.PR_NUMBER and returns True only when pr_number > 0. On master/release runs (pr_number <= 0: push, schedule, dispatch, merge-queue) a missing artifact is still an error even when optional, so that shard job reddens, the run fails noisy, and the final merge never publishes an incomplete llvm_coverage.info. Non-optional artifacts error on any run, as before.
Regression tests lock this in: test_runner_does_not_skip_missing_optional_artifact_on_master (asserts pr_number 0 and -1 both error) alongside the PR-run skip case and the non-optional error case.
The optional flag on the LLVM coverage profdata artifacts made a missing .profdata non-blocking on every workflow, including master. But master must publish a complete llvm_coverage.info baseline (see ci/jobs/scripts/workflow_hooks/filter_job.py): if a master shard's non-blocking llvm-profdata merge crashes and produces no file, staying green would upload a partial baseline and later PRs would diff against it and report artificial coverage deltas. Gate the skip to PR runs (env.PR_NUMBER > 0). On master/release runs a missing artifact is still an error even when optional, so master stays fail-noisy and the baseline stays complete. PR coverage jobs keep the non-blocking behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Updated the fix per the review (commit 814773d): the optional-artifact skip is now gated to PR runs so master keeps a complete coverage baseline. Pre-PR validation gate (click to expand)
Session id: cron:clickhouse-worker-slot-8:20260703-074200 |
CI finish ledger — 814773dCI fully finished on head
Session id: cron:our-pr-ci-monitor:20260707-040000 |
|
Suggest naming the flag |
leshikus
left a comment
leshikus
left a comment
There was a problem hiding this comment.
Other than the name, looks good
@groeneai , better to make it optional for all cases |
Pull request was converted to draft
Per reviewer decision: a missing optional providing artifact is skipped with a warning on any run (PR, master or release), not gated to PR runs. The flag stays named optional. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Done in cf8cb51. The runner now skips a missing |
Pre-PR validation gate (click to expand)
Session id: cron:clickhouse-worker-slot-6:20260708-123000 |
CI finish ledger — cf8cb51CI fully finished, all green. No failed checks.
Session id: cron:our-pr-ci-monitor:20260708-143000 |
robot-ch-test-poll2
added
the
pr-synced-to-cloud
…optional-profdata-artifact Do not fail coverage jobs when the profdata artifact is missing
5 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
...
Description
Problem.
Integration tests (amd_llvm_coverage, N/8)jobs (and the Functional / Unit test coverage siblings) intermittently finish with job-levelcheck_status=errorwhile every test row isOKand there is no timeout row. On the affected runstest_context_rawis justFailures: 0/625.Root cause. The LLVM coverage jobs run their coverage merge non-blocking (
force_ok_exit=True, "do not block pipeline").llvm-profdata mergecan crash (SIGSEGV) while reading a corrupt.profrawheader:merge_profraw_files()then returnsNoneand writes no./*.profdata. So far the job is still green. But each coverage job also declaresprovides=[LLVM_COVERAGE_FILE_*]withpath=['./*.profdata'], and the praktika post-run upload treats a missing providing artifact as a hard error:result.set_status(Result.Status.ERROR)reddens the whole job even though 0 tests failed. The "do not block pipeline" intent only made the test result non-blocking; the mandatoryprovides-artifact upload runs afterward and hard-fails on the missing file.Fix. Add an
optionalflag toArtifact.Config. When a providing artifact marked optional matches no file at upload time, the runner skips it with a warning instead of settingERROR. The coverage profdata artifacts are marked optional. This is safe because the downstreamLLVM Coverageaggregation job already globs whatever.profdatafiles exist (llvm-profdata merge -sparse -failure-mode=warn *.profdata,--ignore-errors), so a missing batch does not break aggregation. Non-optional artifacts (build binaries, etc.) keep the old strict behavior.A regression test in
ci/tests/test_optional_artifact.pyasserts that the coverage profdata artifacts are optional, thatoptionaldefaults toFalse(backward compatible), and that the runner skips a missing optional artifact but still errors on a missing required one.Version info
26.7.1.669(included in26.7and later)