From bf689d80cf0b7f5e23b54e523c40139fcefc3941 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Mon, 3 Aug 2026 12:43:31 +0100 Subject: [PATCH] ci(gate): the aggregate check fails when path detection does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI Gate is one of the two checks develop and main require, and it watched the four heavy jobs without watching the path-detection job they all gate on. When that job's git fetch returned HTTP 503 on run 30796472533 the four resolved to skipped rather than failure, so the gate found nothing to report and went green over a run that compiled nothing — both required checks green, and on a pull request that is branch protection satisfied by a build that never happened. The gate now aggregates the detection job. CiGateCoverageGuardTest reads the workflow and fails if any job that can run on a pull request is missing from that list; a schedule-only job is recognised from its own if: condition rather than an exclusion list, so a new job either joins the gate or fails the guard. A second case fails when the gate names a job that no longer exists, which is what a rename would leave behind. ./mvnw -B -ntp clean verify — BUILD SUCCESS. Both new cases confirmed red first: dropping `changes` from the needs list reports ["changes"] unwatched, and misspelling it reports the stale id. --- .github/workflows/ci.yml | 10 +- CHANGELOG.md | 11 ++ .../CiGateCoverageGuardTest.java | 159 ++++++++++++++++++ 3 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3459774c..fbc853b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,7 +55,7 @@ jobs: # in qa) run in build-and-test below, which now also covers docs-only PRs. run: | ./mvnw -B -ntp clean \ - "-Dtest=EnginePdfBoundaryTest,DocumentationCoverageTest,CanonicalSurfaceGuardTest,PackageMapGuardTest,VersionConsistencyGuardTest,CiGuardListGuardTest,CodeQlScopeGuardTest" \ + "-Dtest=EnginePdfBoundaryTest,DocumentationCoverageTest,CanonicalSurfaceGuardTest,PackageMapGuardTest,VersionConsistencyGuardTest,CiGuardListGuardTest,CiGateCoverageGuardTest,CodeQlScopeGuardTest" \ test -pl :graph-compose-core changes: @@ -435,8 +435,14 @@ jobs: # "Architecture and Documentation Guards") instead of the individual, sometimes # -skipped matrix legs so a docs-only PR is never left waiting on a check that # never reports. + # + # `changes` is in the list even though it decides rather than builds. When its + # fetch fails the heavy jobs downstream resolve to `skipped`, not `failure`, so + # a gate that watched only them stayed green over a run where nothing was built. + # Every job that can run on a pull request belongs here; only the schedule-only + # benchmark job is outside, and CiGateCoverageGuardTest holds that line. if: always() && github.event_name != 'schedule' - needs: [architecture-and-documentation-guards, build-and-test, examples-generation, binary-compat, perf-smoke] + needs: [architecture-and-documentation-guards, changes, build-and-test, examples-generation, binary-compat, perf-smoke] runs-on: ubuntu-latest steps: - name: Fail if any required job failed diff --git a/CHANGELOG.md b/CHANGELOG.md index 47564c63..eab3eb9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,17 @@ follow semantic versioning; release dates are ISO 8601. four that did match kept the job green. The list is now the five guards that live in the engine module, and `CiGuardListGuardTest` fails the job if a name in it stops resolving. +- **The aggregate status check notices when nothing was built.** `CI Gate` is one of + the two checks `develop` and `main` require, and it watched the four heavy jobs + without watching the path-detection job they all gate on. When that job's + `git fetch` returned HTTP 503 the four resolved to `skipped` rather than `failure`, + so the gate found nothing to report and went green over a run that compiled + nothing — leaving both required checks green and, on a pull request, the branch + protection satisfied by a build that never happened. The gate now aggregates the + detection job too, and `CiGateCoverageGuardTest` reads the workflow and fails if + any job that can run on a pull request is left out of it. Schedule-only jobs are + recognised from their own `if:` condition rather than an exclusion list, so a new + job either joins the gate or fails the guard. - **A documentation-only pull request is compiled.** Markdown was not a change-detection input, so a PR touching only `.md` skipped the reactor build and merged without `DocumentationSnippetCompileTest` ever compiling the java diff --git a/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java b/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java new file mode 100644 index 00000000..e77d69bb --- /dev/null +++ b/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java @@ -0,0 +1,159 @@ +package com.demcha.documentation; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards that the aggregate CI status check watches every job that can run on a + * pull request. + * + *

{@code CI Gate} exists so branch protection has one stable check to require + * instead of matrix legs the path filter sometimes skips. It fails when a job it + * {@code needs} reports {@code failure} or {@code cancelled}; a job the filter + * legitimately skipped counts as success, which is the whole point.

+ * + *

That design has one hole, and it is not hypothetical. {@code changes} — the + * path-detection job every heavy job gates on — was absent from the gate's + * {@code needs}. When its {@code git fetch} returned HTTP 503 the job failed, the + * four reactor jobs downstream resolved to {@code skipped} rather than + * {@code failure}, and the gate reported success over a run that compiled nothing. + * Both checks branch protection is pointed at were green with zero tests + * executed.

+ * + *

So the rule is: every job in the workflow is either aggregated by the gate or + * demonstrably cannot run on a pull request. The second case is derived from the + * job's own {@code if:} condition rather than a list somebody maintains, so a new + * job joins the gate or fails this test — it cannot slip through by being + * forgotten.

+ */ +class CiGateCoverageGuardTest { + + private static final Path PROJECT_ROOT = RepoRoot.get(); + private static final Path WORKFLOW = PROJECT_ROOT.resolve(".github/workflows/ci.yml"); + + /** The aggregate check. It cannot depend on itself. */ + private static final String GATE = "ci-gate"; + + /** A job key: two-space indent under {@code jobs:}, nothing else on the line. */ + private static final Pattern JOB_KEY = Pattern.compile("(?m)^ ([a-z][a-z0-9-]*):$"); + + /** A job-level {@code if:} — four-space indent, first line only. */ + private static final Pattern JOB_IF = Pattern.compile("(?m)^ if: (.*)$"); + + /** An inline {@code needs: [a, b, c]} flow sequence. */ + private static final Pattern JOB_NEEDS = Pattern.compile("(?m)^ needs: \\[([^]]*)]"); + + /** + * A job that only runs on a schedule or a manual dispatch never appears on a + * pull request, so the gate has nothing to aggregate from it. + */ + private static final Pattern SCHEDULE_ONLY = Pattern.compile("github\\.event_name == 'schedule'"); + + @Test + void ciGateAggregatesEveryJobThatCanRunOnAPullRequest() throws IOException { + Map jobs = jobBlocks(); + + assertThat(jobs) + .describedAs("no jobs parsed out of %s — this guard is reading a workflow shape " + + "that moved, so it is no longer guarding anything", relative(WORKFLOW)) + .isNotEmpty() + .containsKey(GATE); + + Set aggregated = needsOf(jobs.get(GATE)); + assertThat(aggregated) + .describedAs("the '%s' job must declare an inline 'needs: [...]' list", GATE) + .isNotEmpty(); + + List unwatched = new ArrayList<>(); + for (Map.Entry job : jobs.entrySet()) { + String id = job.getKey(); + if (id.equals(GATE) || runsOnlyOnASchedule(job.getValue()) || aggregated.contains(id)) { + continue; + } + unwatched.add(id); + } + + assertThat(unwatched) + .describedAs("every job that can run on a pull request must be in the '%s' needs " + + "list, or a failure there leaves the aggregate check green over a run " + + "that built nothing", GATE) + .isEmpty(); + } + + @Test + void ciGateDoesNotDependOnAJobThatIsGone() throws IOException { + Map jobs = jobBlocks(); + Set aggregated = needsOf(jobs.get(GATE)); + + assertThat(jobs.keySet()) + .describedAs("'%s' aggregates a job id that no longer exists: the workflow would " + + "fail to load, and the rename that caused it is the finding", GATE) + .containsAll(aggregated); + } + + /** Job id to the source block that declares it, in workflow order. */ + private static Map jobBlocks() throws IOException { + // The workflow is checked out with CRLF on Windows. Normalise once, so the + // line-anchored patterns below capture ids and conditions without a trailing + // carriage return riding along into every comparison. + String workflow = Files.readString(WORKFLOW).replace("\r\n", "\n"); + // `on:` carries keys at the same indentation as a job (`push:`, `schedule:`), + // so parsing starts after the `jobs:` key rather than at the top of the file. + int jobsAt = workflow.indexOf("\njobs:\n"); + assertThat(jobsAt) + .describedAs("no top-level 'jobs:' key in %s", relative(WORKFLOW)) + .isNotNegative(); + String body = workflow.substring(jobsAt); + + Map blocks = new LinkedHashMap<>(); + Matcher key = JOB_KEY.matcher(body); + List ids = new ArrayList<>(); + List starts = new ArrayList<>(); + while (key.find()) { + ids.add(key.group(1)); + starts.add(key.end()); + } + for (int i = 0; i < ids.size(); i++) { + int end = i + 1 < ids.size() ? starts.get(i + 1) : body.length(); + blocks.put(ids.get(i), body.substring(starts.get(i), end)); + } + return blocks; + } + + private static Set needsOf(String jobBlock) { + Matcher needs = JOB_NEEDS.matcher(jobBlock); + if (!needs.find()) { + return Set.of(); + } + Set ids = new LinkedHashSet<>(); + for (String id : needs.group(1).split(",")) { + String trimmed = id.trim(); + if (!trimmed.isEmpty()) { + ids.add(trimmed); + } + } + return ids; + } + + private static boolean runsOnlyOnASchedule(String jobBlock) { + Matcher condition = JOB_IF.matcher(jobBlock); + return condition.find() && SCHEDULE_ONLY.matcher(condition.group(1)).find(); + } + + private static String relative(Path path) { + return PROJECT_ROOT.relativize(path).toString().replace('\\', '/'); + } +}