From aed83aae4f82dccd52034f3d80a9d775fbea3797 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Fri, 31 Jul 2026 03:56:29 +0100 Subject: [PATCH 1/2] ci(guards): run every guard the job names, and compile docs-only pull requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Architecture and Documentation Guards" job selected eight test classes while scoping the reactor to graph-compose-core. Two had been deleted months earlier; two live in graph-compose-qa and graph-compose-render-pdf. Surefire aborts only when a selection is entirely empty, so the four names that did match kept the job green while it ran half of what it advertised. The list is now the five guards that live in the engine module, and CiGuardListGuardTest parses the workflow and fails the job when a name in it stops resolving under core/src/test/java — inside the very job it protects. Markdown was also missing from change detection, so a pull request touching only .md skipped the reactor build entirely and merged without DocumentationSnippetCompileTest ever compiling the java fences it publishes. A separate `docs` filter routes those changes into the reactor slice that carries the qa guards, on the baseline JDK; example generation stays gated on `code`, since rendering the catalogue proves nothing about prose. PackageMapGuardTest scanned gitignored docs/private/, where a local note routinely names a retired package in order to record that it is retired. A hit there fails the release runbook's local verify gate for a reason CI can never reproduce, so the directory is excluded — as CanonicalSurfaceGuardTest already excludes it. Three workflow comments claimed things that are not true: that render-docx, render-pptx and graph-compose-testing are absent from Maven Central (they are published; the examples pin them to the reactor's -SNAPSHOT), that the pptx module is a semantic backend (its primary backend is fixed-layout), and that the CodeQL scope matches the canonical verify gate (it is narrower, and the render backends ship unscanned). graph-compose-render-pptx compiles against org.apache.pdfbox types while declaring only fontbox; the dependency is now explicit. Both modules pin pdfbox.version to 3.0.8, so the resolved version is unchanged. --- .github/workflows/ci.yml | 66 ++++++--- .github/workflows/codeql.yml | 12 +- .github/workflows/publish.yml | 6 +- CHANGELOG.md | 18 +++ .../documentation/CiGuardListGuardTest.java | 137 ++++++++++++++++++ .../documentation/PackageMapGuardTest.java | 8 + render-pptx/pom.xml | 10 ++ 7 files changed, 232 insertions(+), 25 deletions(-) create mode 100644 core/src/test/java/com/demcha/documentation/CiGuardListGuardTest.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 84bf9f78..0f716b0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,16 +45,24 @@ jobs: java-version: '17' cache: maven - - name: Run guards + - name: Run core architecture and documentation guards + # Every name here must live in graph-compose-core: the run is scoped to + # that module, and Surefire only fails on an empty selection — a name + # that matches nothing while its siblings match is dropped in silence. + # CiGuardListGuardTest asserts each name resolves under core/src/test. + # Guards owned by other modules (PdfRenderInterfaceGuardTest in + # render-pdf, DocumentationExamplesTest and DocumentationSnippetCompileTest + # in qa) run in build-and-test below, which now also covers docs-only PRs. run: | ./mvnw -B -ntp clean \ - "-Dtest=EnginePdfBoundaryTest,CanonicalTemplateComposerPdfBoundaryTest,PdfRenderInterfaceGuardTest,DocumentationCoverageTest,DocumentationExamplesTest,CanonicalSurfaceGuardTest,TemplateComposeApiTest,VersionConsistencyGuardTest" \ + "-Dtest=EnginePdfBoundaryTest,DocumentationCoverageTest,CanonicalSurfaceGuardTest,PackageMapGuardTest,VersionConsistencyGuardTest" \ test -pl :graph-compose-core changes: # Path-based change detection for selective CI on pull requests. Emits the # reverse-dependency flags the heavy jobs gate on: `code` (any build input), - # `core` (the root graph-compose-core module — drives japicmp), `perf` + # `docs` (markdown the guard suites read and compile), `core` (the root + # graph-compose-core module — drives japicmp), `perf` # (modules the smoke benchmark exercises), and `jvm` (published library modules # + toolchain — drives the JDK matrix width). Pushes/dispatch bypass these gates. name: Detect changed paths @@ -62,6 +70,7 @@ jobs: runs-on: ubuntu-latest outputs: code: ${{ steps.filter.outputs.code }} + docs: ${{ steps.filter.outputs.docs }} core: ${{ steps.filter.outputs.core }} perf: ${{ steps.filter.outputs.perf }} jvm: ${{ steps.filter.outputs.jvm }} @@ -86,6 +95,16 @@ jobs: - 'mvnw' - 'mvnw.cmd' - '.github/workflows/ci.yml' + # Markdown is a build input even though it compiles nothing: + # DocumentationSnippetCompileTest compiles the literal java fences in + # docs/, and the guard suites read README / CONTRIBUTING / docs. Kept + # separate from `code` so a docs-only change runs the reactor (which + # carries those guards) without also running examples-generation, + # which installs eight modules and renders the full example catalogue + # to prove nothing about prose. Deliberately absent from `jvm` too, so + # a docs-only PR uses the baseline JDK alone rather than the matrix. + docs: + - '**/*.md' core: - 'core/src/**' - 'core/pom.xml' @@ -119,9 +138,12 @@ jobs: build-and-test: name: Build and run tests (JDK ${{ matrix.java }}) # Selective CI: on a pull request, skip the heavy reactor build + JDK matrix - # when only docs / non-build files changed (`code` is false). Pushes to the - # integration branches and manual dispatch always run the full gate. - if: github.event_name != 'schedule' && (github.event_name != 'pull_request' || needs.changes.outputs.code == 'true') + # when nothing this reactor slice can check has changed. Markdown counts — + # this slice carries the qa guards that compile the published snippets, so a + # docs-only PR runs here (on the baseline JDK alone, since `jvm` stays false) + # rather than merging without any compiler having read the change. Pushes to + # the integration branches and manual dispatch always run the full gate. + if: github.event_name != 'schedule' && (github.event_name != 'pull_request' || needs.changes.outputs.code == 'true' || needs.changes.outputs.docs == 'true') needs: [architecture-and-documentation-guards, changes] runs-on: ubuntu-latest strategy: @@ -158,7 +180,9 @@ jobs: # only via the examples job), the extracted graph-compose-testing module, the # graph-compose-qa cross-module suites (which need the testing module + the # engine test-jar on the classpath and so cannot live in the engine's own test - # scope), and the graph-compose-coverage aggregator (JaCoCo over core + + # scope — this is where DocumentationSnippetCompileTest, the guard that + # compiles the literal markdown fences in docs/, and DocumentationExamplesTest + # actually run), and the graph-compose-coverage aggregator (JaCoCo over core + # render-pdf + templates, counting the qa exec, now with a NON-REGRESSION # RATCHET — this step fails if aggregate INSTRUCTION or BRANCH coverage drops # below the floor in coverage/pom.xml). -am pulls the fonts / emoji upstreams; @@ -189,7 +213,9 @@ jobs: examples-generation: name: Examples Generation Smoke Test - # Follows build-and-test: skipped for a docs-only PR, runs otherwise. + # Gated on `code` alone, deliberately narrower than build-and-test above: + # rendering the example catalogue costs eight module installs and proves + # nothing about a markdown change, so a docs-only PR skips it. if: github.event_name != 'schedule' && (github.event_name != 'pull_request' || needs.changes.outputs.code == 'true') needs: [build-and-test, changes] runs-on: ubuntu-latest @@ -214,9 +240,10 @@ jobs: run: ./mvnw -B -ntp -f fonts/pom.xml -DskipTests install - name: Install graph-compose-emoji (consumed by the examples module) - # The emoji example renders colour emoji from the bundled Noto SVG set; - # like graph-compose-fonts it is a standalone artifact not on Maven - # Central, so install it into the local repo before building examples. + # The emoji example renders colour emoji from the bundled Noto SVG set. + # Like graph-compose-fonts it carries its own version line, so the reactor + # slice never builds it; installing from source pins the examples to this + # tree's asset set rather than the cached jar at the same coordinate. run: ./mvnw -B -ntp -f emoji/pom.xml -DskipTests install - name: Install root artifact (graph-compose-core) @@ -235,22 +262,23 @@ jobs: run: ./mvnw -B -ntp -f wrapper/pom.xml -DskipTests install - name: Install graph-compose-render-docx (consumed by the examples module) - # The DOCX export example depends on the render-docx backend module, which - # is not on Central. Install it after the engine (its graph-compose - # dependency) so the examples build resolves it. + # The DOCX export example depends on the render-docx backend module at + # ${graphcompose.version} — the reactor's -SNAPSHOT, which no repository + # can supply. Install it after the engine (its graph-compose dependency) + # so the examples build resolves it. run: ./mvnw -B -ntp -f render-docx/pom.xml -DskipTests install - name: Install graph-compose-testing (consumed by the examples module) # LayoutSnapshotRegressionExample uses LayoutSnapshotJson and EngineDeckData - # reads benchmark JSON through the jackson it brings; the testing module is - # not on Central, so install it after the engine (its graph-compose - # dependency) before the examples build resolves it. + # reads benchmark JSON through the jackson it brings; the examples pin the + # testing module to the reactor's -SNAPSHOT, so install it after the engine + # (its graph-compose dependency) before the examples build resolves it. run: ./mvnw -B -ntp -f testing/pom.xml -DskipTests install - name: Install graph-compose-render-pptx (consumed by the examples module) # The Engine Deck PPTX example renders the flagship deck through the - # fixed-layout PPTX backend, which is not on Central. Install it after - # the wrapper (its core + render-pdf compile dependencies) AND the + # fixed-layout PPTX backend, pinned to the reactor's -SNAPSHOT version. + # Install it after the wrapper (its core + render-pdf compile dependencies) AND the # testing module — -DskipTests skips execution but Maven still # resolves render-pptx's test-scope graph-compose-testing dependency. run: ./mvnw -B -ntp -f render-pptx/pom.xml -DskipTests install diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 6bea0b70..388a40ca 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -50,10 +50,14 @@ jobs: languages: ${{ matrix.language }} queries: security-and-quality - - name: Build (library module only) - # `-pl :graph-compose-core` scopes the analysis to the published artifact and matches - # the canonical verify gate. Examples/benchmarks are intentionally - # excluded — they ship no production code. + - name: Build (engine module) + # Scoped to the engine module alone: no `-am`, so nothing but core is built + # and nothing but core reaches the extractor. Since the 2.0 split + # graph-compose-core is one of several artifacts published to Maven Central, + # which makes this narrower than both the publish train and the canonical + # verify gate in ci.yml — the render backends and templates ship unscanned. + # Widening it to the code-bearing published modules is tracked for 2.2, + # together with a triage pass on the standing alert set. run: ./mvnw -B -ntp -DskipTests -pl :graph-compose-core package - name: Perform CodeQL analysis diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 0a3b9014..8c2f1af0 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -226,8 +226,10 @@ jobs: - name: Publish render-pptx to Maven Central if: steps.plan.outputs.run_render_pptx == 'true' - # The semantic PPTX backend, lockstep-versioned with the engine (ships on - # the same v* tag). graph-compose-core resolves from the local repo installed + # The PPTX render backend — a fixed-layout POI XSLF backend consuming the + # same resolved LayoutGraph as the PDF one, alongside the older semantic + # manifest skeleton. Lockstep-versioned with the engine (ships on the same + # v* tag). graph-compose-core resolves from the local repo installed # by the engine deploy above. run: ./mvnw -B -ntp -f render-pptx/pom.xml -P release -DskipTests -Dgpg.skip=false deploy env: diff --git a/CHANGELOG.md b/CHANGELOG.md index 558e6688..eaaa8ee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ follow semantic versioning; release dates are ISO 8601. ## v2.1.1 — Planned +### Build + +- **The CI guard job runs every guard it names.** It selected eight test classes + while scoping the reactor to `graph-compose-core`: two had been deleted months + earlier and two live in `graph-compose-qa` and `graph-compose-render-pdf`, so + four never ran. Surefire aborts only when a selection is entirely empty, so the + 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. +- **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 + fences it publishes. Markdown now routes to the reactor slice that carries those + guards, on the baseline JDK, and still skips example generation. +- **`graph-compose-render-pptx` declares PDFBox.** It compiles against + `org.apache.pdfbox` types while declaring only `fontbox`, taking the rest + transitively; the resolved version is unchanged. + ### Fixed - **A heading no longer strands above a block that was asked to stay whole.** diff --git a/core/src/test/java/com/demcha/documentation/CiGuardListGuardTest.java b/core/src/test/java/com/demcha/documentation/CiGuardListGuardTest.java new file mode 100644 index 00000000..5555f6a6 --- /dev/null +++ b/core/src/test/java/com/demcha/documentation/CiGuardListGuardTest.java @@ -0,0 +1,137 @@ +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.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards the hand-written {@code -Dtest=} list in the CI guard job against name rot. + * + *

The "Architecture and Documentation Guards" job selects a fixed set of test + * classes by name and runs them with {@code -pl :graph-compose-core}. Surefire + * treats that selection as a whole: it aborts only when every pattern + * matches nothing. A single name that matches nothing while its siblings match is + * dropped without a warning, and the job reports success having run fewer tests + * than it names.

+ * + *

Two failure modes therefore pass CI silently. A guard renamed or deleted + * leaves a dead name behind — the job keeps claiming coverage it no longer has. + * A guard that lives in another module ({@code qa}, {@code render-pdf}) can be + * added to the list in good faith and never run, because the module scope + * excludes it. Both had happened here: the list carried two classes deleted + * months earlier plus two that live outside {@code core}, so the job executed + * four of the eight names it advertised.

+ * + *

This test parses the workflow and asserts every selected name resolves to a + * test source under {@code core/src/test/java}. It runs inside the very job it + * protects, so a bad edit to that line fails the step that made it. Guards owned + * by other modules belong in the reactor build, not here.

+ */ +class CiGuardListGuardTest { + + private static final Path PROJECT_ROOT = RepoRoot.get(); + private static final Path WORKFLOW = PROJECT_ROOT.resolve(".github/workflows/ci.yml"); + private static final Path CORE_TESTS = PROJECT_ROOT.resolve("core/src/test/java"); + + /** + * The scope that makes a {@code -Dtest=} selection core-only. Anchored at the + * end of the reactor list so {@code -pl :graph-compose-core,:graph-compose-qa} + * — a genuinely multi-module selection — is not held to core residency. + */ + private static final Pattern CORE_SCOPE = + Pattern.compile("-pl :graph-compose-core(?![\\w:,-])"); + + /** A {@code -Dtest=} value, quoted or bare, up to the end of the shell word. */ + private static final Pattern TEST_SELECTION = Pattern.compile("-Dtest=\"?([A-Za-z0-9_$.,*#\\[\\]-]+)\"?"); + + /** Workflow steps start at a {@code - name:} key at any indentation. */ + private static final Pattern STEP_BOUNDARY = Pattern.compile("(?m)^\\s*- name:"); + + @Test + void everyGuardNamedInTheCoreScopedCiStepLivesInCore() throws IOException { + List selected = coreScopedTestSelections(); + + assertThat(selected) + .describedAs("no core-scoped '-Dtest=' step found in %s — this guard is reading a " + + "workflow shape that moved, so it is no longer guarding anything", + relative(WORKFLOW)) + .isNotEmpty(); + + Set missing = new TreeSet<>(); + for (String name : selected) { + if (!resolvesUnderCoreTests(name)) { + missing.add(name); + } + } + + assertThat(missing) + .describedAs("every class named in the core-scoped '-Dtest=' list of %s must exist " + + "under core/src/test/java. Surefire drops an unmatched name in silence " + + "whenever another name in the same selection matches, so a deleted guard " + + "— or one that lives in qa / render-pdf — leaves the job reporting " + + "coverage it does not have. Guards owned by another module run in the " + + "reactor build instead.", relative(WORKFLOW)) + .isEmpty(); + } + + /** + * Class names selected by every workflow step that both filters with + * {@code -Dtest=} and scopes the reactor to {@code graph-compose-core}. A step + * that selects tests for a different module is intentionally not checked here. + */ + private static List coreScopedTestSelections() throws IOException { + String workflow = Files.readString(WORKFLOW); + List names = new ArrayList<>(); + + for (String step : STEP_BOUNDARY.split(workflow)) { + if (!CORE_SCOPE.matcher(step).find()) { + continue; + } + Matcher selection = TEST_SELECTION.matcher(step); + while (selection.find()) { + Arrays.stream(selection.group(1).split(",")) + .map(String::trim) + .filter(name -> !name.isEmpty()) + .forEach(names::add); + } + } + return new ArrayList<>(new LinkedHashSet<>(names)); + } + + /** + * Whether a Surefire selector names a test source in the engine module. The + * {@code #method} suffix is stripped and a package-qualified name is resolved + * as a path, so neither form can smuggle a class name past the check. Only a + * wildcard selector is accepted unchecked — it matches by shape, not by name, + * and has nothing to resolve. + */ + private static boolean resolvesUnderCoreTests(String selector) throws IOException { + String className = selector.split("#", 2)[0]; + if (className.contains("*")) { + return true; + } + if (className.contains(".")) { + return Files.isRegularFile(CORE_TESTS.resolve(className.replace('.', '/') + ".java")); + } + try (Stream sources = Files.walk(CORE_TESTS)) { + return sources.anyMatch(path -> path.getFileName().toString().equals(className + ".java")); + } + } + + private static String relative(Path path) { + return PROJECT_ROOT.relativize(path).toString().replace('\\', '/'); + } +} diff --git a/core/src/test/java/com/demcha/documentation/PackageMapGuardTest.java b/core/src/test/java/com/demcha/documentation/PackageMapGuardTest.java index f50b456c..60f00e64 100644 --- a/core/src/test/java/com/demcha/documentation/PackageMapGuardTest.java +++ b/core/src/test/java/com/demcha/documentation/PackageMapGuardTest.java @@ -114,6 +114,14 @@ private boolean isIgnored(Path path) { return relative.startsWith(".git/") || relative.startsWith(".claude/") || relative.startsWith(".idea/") + // docs/private/ is gitignored — local-only planning notes and + // audits, not part of the public docs surface. They routinely + // name a retired package in order to record that it is retired, + // which reads as a violation here. Because the directory never + // reaches the remote, a hit fails the release runbook's local + // verify gate for a reason CI can never reproduce or clear. + // CanonicalSurfaceGuardTest excludes it for the same reason. + || relative.startsWith("docs/private/") || relative.contains("/target/") || relative.startsWith("target/") || relative.startsWith("logs/"); diff --git a/render-pptx/pom.xml b/render-pptx/pom.xml index 9887b6ae..71233a24 100644 --- a/render-pptx/pom.xml +++ b/render-pptx/pom.xml @@ -97,6 +97,16 @@ fontbox ${pdfbox.version} + + + org.apache.pdfbox + pdfbox + ${pdfbox.version} +