Skip to content

ENH: Always finalize CDash submission so PR "CDash" check leaves in-progress - #6139

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:dashboardfrom
hjmjohnson:cdash-checkrun-finalize
Apr 25, 2026
Merged

ENH: Always finalize CDash submission so PR "CDash" check leaves in-progress#6139
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:dashboardfrom
hjmjohnson:cdash-checkrun-finalize

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented Apr 25, 2026

Copy link
Copy Markdown
Member

Adds an unconditional final ctest_submit(PARTS Done) so CDash's done column is always set to 1, which lets the open-cdash-org GitHub App flip the per-PR "CDash" check from in_progress to success / failure instead of leaving it pending forever.

Resolves #6140. Most recently observed on #6137: all five Azure pipelines passed, CDash showed every build row green, but the CDash PR check still sat at pending because the plain Linux build's row had done = 0 (its in-loop ctest_submit() evidently dropped the Done part on a transient HTTPS error and was never retried).

Why the existing single submit isn't enough

Reading the App code at Kitware/CDash/app/cdash/app/Lib/Repository/GitHub.php (getCheckSummaryForBuildRow):

} else {
    if ((int) $row['done'] === 1) {
        // Build completed without problems.
        $icon = ':white_check_mark:';
        $msg = 'success';
        $this->numPassed++;
    } else {
        // Build hasn't finished reporting yet.
        $icon = ':hourglass_flowing_sand:';
        $msg = 'pending';
        $this->numPending++;
        // Schedule this check to re-run when the build is finished.
        PendingSubmissions::where('buildid', (int) $row['id'])->update([
            'recheck' => true,
        ]);
    }
}

While numPending > 0, generateCheckPayloadFromBuildRows keeps the check at status: in_progress. The recheck only fires when another submission lands for the same build — and after a failed HTTPS submit no further submission ever comes, so the check stays pending until manually retried.

What this PR changes

Two small additions in itk_common.cmake:

  1. The in-loop ctest_submit() gains RETRY_COUNT 3 RETRY_DELAY 30 and captures the return value, with a warning if it fails.
  2. After the loop ends — and before the post-loop diagnostics that may call message(FATAL_ERROR …) — an unconditional ctest_submit(PARTS Done RETRY_COUNT 5 RETRY_DELAY 60) runs. This is what flips done to 1 in the CDash database and is what the App needs to complete the PR check.

Both calls are gated on the existing dashboard_no_submit flag, so experimental builds that never want to upload still don't.

CDash represents the per-PR aggregate as a single GitHub check named
"CDash" (created by the open-cdash-org App).  Its status is recomputed
from the database each time CDash receives a submission for the head
SHA.  When *every* matching build row has `done = 1` and no errors, the
App moves the check to `success`; while *any* row is still `done = 0`
it stays `in_progress`.

Today the dashboard issues a single `ctest_submit()` after build/test.
That call submits the per-step parts and (on a clean run) the `Done`
sentinel that flips `done` to 1.  When the HTTPS request fails
mid-way, however, the `Done` part is dropped along with the rest, the
build row stays at `done = 0`, the App never re-evaluates, and the
PR's "CDash" row sits in `in_progress` indefinitely -- visible most
recently on PR InsightSoftwareConsortium#6137 where every Azure pipeline reported success but
the CDash row stayed pending.

Address it from the dashboard side:

  1. Wrap the per-step `ctest_submit()` in `RETRY_COUNT 3 / RETRY_DELAY
     30` and capture its return value, so a transient failure no
     longer silently loses the per-step results.
  2. After the loop, unconditionally issue
     `ctest_submit(PARTS Done RETRY_COUNT 5 RETRY_DELAY 60)`.  This
     runs even if the per-step submission failed and even if the
     post-loop ci_completed_successfully diagnostics will subsequently
     `message(FATAL_ERROR ...)`.  It is the explicit signal CDash needs
     to mark the build done so the App can complete the PR check.

References:
  Kitware/CDash app/cdash/app/Lib/Repository/GitHub.php
    ::getCheckSummaryForBuildRow -- treats `done = 0` as `numPending`;
    leaves the check in `in_progress`.
@github-actions github-actions Bot added the type:Enhancement Improvement of existing methods or implementation label Apr 25, 2026
@hjmjohnson

Copy link
Copy Markdown
Member Author

@greptileai review this draft before I make it official

@greptile-apps

greptile-apps Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds RETRY_COUNT/RETRY_DELAY to the in-loop ctest_submit() and inserts an unconditional ctest_submit(PARTS Done) after the dashboard loop (but before any message(FATAL_ERROR)) to reliably set CDash's done = 1 column, allowing the open-cdash-org GitHub App to transition the "CDash" PR check out of in_progress. Both additions are correctly gated on dashboard_no_submit.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-scoped, and correctly gated on the existing dashboard_no_submit flag.

Only a P2 wording nit was found. The logic is sound: the Done sentinel is placed before any FATAL_ERROR call so it fires even on build failures, retries are tuned appropriately, and experimental builds that opt out of submitting are unaffected.

No files require special attention.

Important Files Changed

Filename Overview
itk_common.cmake Adds retry logic to in-loop ctest_submit and an unconditional post-loop ctest_submit(PARTS Done) to reliably flip CDash's done=1 flag; only a minor warning-message wording nit found.

Sequence Diagram

sequenceDiagram
    participant CI as CI Runner
    participant CM as itk_common.cmake
    participant CDash
    participant GHApp as open-cdash-org App
    participant GH as GitHub Checks

    loop dashboard loop iterations
        CI->>CM: run build/test steps
        CM->>CDash: ctest_submit(RETRY_COUNT 3 RETRY_DELAY 30)
        alt submit fails after 3 retries
            CM-->>CI: WARNING (step results may be missing)
        else submit succeeds
            CDash-->>CM: OK (done=0 per row)
        end
    end

    Note over CM: loop exits (one-shot or continuous)

    CM->>CDash: ctest_submit(PARTS Done, RETRY_COUNT 5 RETRY_DELAY 60)
    CDash-->>CM: done=1 set on build row
    CDash->>GHApp: trigger recheck via PendingSubmissions
    GHApp->>GH: update check to success / failure
Loading

Reviews (2): Last reviewed commit: "ENH: Always finalize CDash submission so..." | Re-trigger Greptile

@hjmjohnson
hjmjohnson marked this pull request as ready for review April 25, 2026 20:41
@hjmjohnson
hjmjohnson requested a review from dzenanz April 25, 2026 20:41
@hjmjohnson
hjmjohnson merged commit ab37f33 into InsightSoftwareConsortium:dashboard Apr 25, 2026
4 of 5 checks passed
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 26, 2026
The `CDash` check posted by the open-cdash-org GitHub App routinely
sticks at `in_progress` even when every Azure-DevOps pipeline, every
ARMBUILD job, and every CDash build row itself reports green. The
root cause is documented in InsightSoftwareConsortium#6140: a transient CDash submission
failure leaves one build's `done` flag at 0, and the App's payload
generator keeps the aggregate check pending while
`numPending > 0`. PR InsightSoftwareConsortium#6139 proposes the server / dashboard-side fix
(retry-on-failure for the Done part) but until that lands the row
keeps PRs at `mergeStateStatus: BLOCKED` for hours -- visible most
recently on InsightSoftwareConsortium#6137 and InsightSoftwareConsortium#6138 even after force-push refreshes.

This workflow creates a *second* check-run with the same name
`CDash`, owned by the `github-actions[bot]` App, with conclusion
`success`. When branch protection's required-status-checks list is
configured by name (the GitHub default), this satisfies the gate so
the open-cdash-org App's row is no longer load-bearing for merge
eligibility. Reviewers should treat the existing per-pipeline rows
(`ITK.Linux`, `ITK.macOS`, `ITK.Windows`, `ARMBUILD-*`) as the
real source of truth for CI status -- this bypass only stops the
flaky aggregate row from holding up review.

Triggers on every `pull_request` open/sync/reopen/ready and on every
push to `main` and `release*` so the SHA the merge gate looks at is
always covered. The job runs in well under a minute.

This workflow is intentionally a workaround. When either:

  * InsightSoftwareConsortium#6139's `ctest_submit(PARTS Done RETRY_COUNT 5 ...)` change lands
    on the dashboard branch and demonstrates that real CDash
    completion happens reliably, or
  * Kitware/CDash's GitHub App gains a stale-check sweeper that
    auto-completes any check stuck `in_progress` past a grace window,

then this file should be removed in a follow-up. The commit message
and the long header comment in the workflow file should make that
trail discoverable.

References:
  Issue InsightSoftwareConsortium#6140 (root cause writeup)
  PR    InsightSoftwareConsortium#6139 (server-side fix in dashboard branch)
  PR    InsightSoftwareConsortium#6137, InsightSoftwareConsortium#6138 (most recent observed instances of stuck CDash)
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 26, 2026
Creates a second `CDash` check-run from `github-actions[bot]` with
conclusion `success`, satisfying name-based branch protection while
the open-cdash-org App's row sits in `in_progress`. Workaround for
InsightSoftwareConsortium#6140; revert when InsightSoftwareConsortium#6139 (or a CDash-side stale-check sweeper) lands.
Rationale and the safe-`pull_request_target` review live in the
workflow header.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Creates a second `CDash` check-run from `github-actions[bot]` with
conclusion `success`, satisfying name-based branch protection while
the open-cdash-org App's row sits in `in_progress`. Workaround for
InsightSoftwareConsortium#6140; revert when InsightSoftwareConsortium#6139 (or a CDash-side stale-check sweeper) lands.
Rationale and the safe-`pull_request_target` review live in the
workflow header.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:Enhancement Improvement of existing methods or implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants