diff --git a/CHANGELOG.md b/CHANGELOG.md index a49a9fb4..c316e5d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,36 @@ follow semantic versioning; release dates are ISO 8601. ### Build +- **The Javadoc gate lints the class readers open first.** It ran with + `subpackages` set to `com.demcha.compose.document`, so `GraphCompose` — the entry + point every snippet in the README starts from — sat in the root package outside it, + and had done since the module layout moved. It carried a real doclint error the + whole time: two `
{@code
* try (DocumentSession document = GraphCompose.document(outputFile)
@@ -51,7 +51,7 @@
* }
* }
*
- * {@code
* try (DocumentSession document = GraphCompose.document()
diff --git a/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardParsingTest.java b/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardParsingTest.java
new file mode 100644
index 00000000..9024b9b5
--- /dev/null
+++ b/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardParsingTest.java
@@ -0,0 +1,105 @@
+package com.demcha.documentation;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Drives {@link CiGateCoverageGuardTest}'s job parser with shapes the repository's
+ * own workflow does not currently contain.
+ *
+ * {@link CiGateCoverageGuardTest} reads {@code .github/workflows/ci.yml}, so it
+ * can only ever exercise the job spellings that file happens to use — today, lower
+ * case with hyphens and nothing after the colon. Every other spelling GitHub
+ * accepts is untested there, and the failure mode is not a red build: a job the
+ * parser does not match is a job the guard cannot report missing from
+ * {@code ci-gate.needs}. It stays absent, the test stays green, and the aggregate
+ * check branch protection requires is blind to it.
+ *
+ * So each spelling gets a workflow of its own here, and each asserts the parser
+ * saw the job at all. These are the cases that would otherwise be discovered by a
+ * new job silently escaping the gate.
+ */
+class CiGateCoverageGuardParsingTest {
+
+ @Test
+ void aJobNameWithAnUnderscoreOrCapitalsIsSeen() {
+ Map jobs = CiGateCoverageGuardTest.jobBlocks("""
+ on: [push]
+
+ jobs:
+ build_and_test:
+ runs-on: ubuntu-latest
+ CodeQL:
+ runs-on: ubuntu-latest
+ """);
+
+ assertThat(jobs).containsKeys("build_and_test", "CodeQL");
+ }
+
+ @Test
+ void aTrailingCommentDoesNotHideTheJob() {
+ Map jobs = CiGateCoverageGuardTest.jobBlocks("""
+ on: [push]
+
+ jobs:
+ security_scan: # runs the scanners
+ runs-on: ubuntu-latest
+ perf-smoke:
+ runs-on: ubuntu-latest
+ """);
+
+ assertThat(jobs)
+ .describedAs("a job whose declaration carries an inline comment must still be "
+ + "parsed — otherwise it can sit outside the gate's needs list with "
+ + "nothing reporting it")
+ .containsKeys("security_scan", "perf-smoke");
+ }
+
+ @Test
+ void trailingWhitespaceDoesNotHideTheJob() {
+ Map jobs = CiGateCoverageGuardTest.jobBlocks(
+ "on: [push]\n\njobs:\n build-and-test: \n runs-on: ubuntu-latest\n");
+
+ assertThat(jobs).containsKey("build-and-test");
+ }
+
+ @Test
+ void keysNestedInsideAJobAreNotMistakenForJobs() {
+ Map jobs = CiGateCoverageGuardTest.jobBlocks("""
+ on: [push]
+
+ jobs:
+ build-and-test:
+ runs-on: ubuntu-latest
+ steps:
+ - name: Compile
+ run: ./mvnw -B verify
+ """);
+
+ assertThat(jobs)
+ .describedAs("only two-space keys are jobs; deeper keys belong to the job above")
+ .containsOnlyKeys("build-and-test");
+ }
+
+ @Test
+ void theBlockOfAJobStopsAtTheNextJob() {
+ Map jobs = CiGateCoverageGuardTest.jobBlocks("""
+ on: [push]
+
+ jobs:
+ first:
+ runs-on: ubuntu-latest
+ if: github.event_name == 'schedule'
+ second:
+ runs-on: ubuntu-latest
+ """);
+
+ assertThat(jobs.get("second"))
+ .describedAs("a job must not inherit the previous job's condition, or the guard "
+ + "excuses it from the gate for a reason that belongs to its neighbour")
+ .doesNotContain("schedule");
+ }
+}
diff --git a/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java b/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java
index e77d69bb..001a0cb9 100644
--- a/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java
+++ b/core/src/test/java/com/demcha/documentation/CiGateCoverageGuardTest.java
@@ -47,8 +47,24 @@ class CiGateCoverageGuardTest {
/** 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 key: two-space indent under {@code jobs:}, then nothing that changes
+ * what the key is — trailing spaces or a {@code #} comment are allowed.
+ *
+ * The name is matched structurally rather than against GitHub's identifier
+ * grammar. A character class admitting only what today's job names happen to
+ * use — lower case and hyphens — drops a job called {@code build_and_test} or
+ * {@code CodeQL} on the floor, and requiring the line to end at the colon drops
+ * {@code security_scan: # nightly}. Either way the guard reports on the jobs it
+ * parsed, and one it never saw is one it cannot report missing from the gate:
+ * the job is absent from {@code needs}, the test is green, and the aggregate
+ * check is blind to it. That is the silence this guard exists to break, so the
+ * pattern takes every key the YAML puts at that level and lets the assertions
+ * decide. {@link #jobBlocks(String)} is package-private for exactly this
+ * reason — {@code CiGateCoverageGuardParsingTest} feeds it the shapes the real
+ * workflow does not currently contain.
+ */
+ private static final Pattern JOB_KEY = Pattern.compile("(?m)^ ([^\\s:#]+):[ \\t]*(?:#.*)?$");
/** A job-level {@code if:} — four-space indent, first line only. */
private static final Pattern JOB_IF = Pattern.compile("(?m)^ if: (.*)$");
@@ -104,12 +120,28 @@ void ciGateDoesNotDependOnAJobThatIsGone() throws IOException {
.containsAll(aggregated);
}
- /** Job id to the source block that declares it, in workflow order. */
+ /** Job id to the source block that declares it, for the workflow on disk. */
private static Map jobBlocks() throws IOException {
+ return jobBlocks(Files.readString(WORKFLOW));
+ }
+
+ /**
+ * Job id to the source block that declares it, in workflow order.
+ *
+ * Package-private, and taking the workflow as text rather than reading it,
+ * so the parser can be driven with job shapes the repository's own workflow
+ * does not happen to contain. A parser that only ever sees valid input it
+ * already handles is one whose blind spots stay theoretical until a new job
+ * lands on one.
+ *
+ * @param workflowText the workflow source
+ * @return each job id mapped to the block that declares it
+ */
+ static Map jobBlocks(String workflowText) {
// 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");
+ String workflow = workflowText.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");
diff --git a/core/src/test/java/com/demcha/documentation/RecipeCatalogueGuardTest.java b/core/src/test/java/com/demcha/documentation/RecipeCatalogueGuardTest.java
new file mode 100644
index 00000000..23e553ae
--- /dev/null
+++ b/core/src/test/java/com/demcha/documentation/RecipeCatalogueGuardTest.java
@@ -0,0 +1,113 @@
+package com.demcha.documentation;
+
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+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;
+
+/**
+ * Every recipe page is reachable from the catalogue that indexes them.
+ *
+ * {@code docs/recipes.md} is the one page the README and the documentation
+ * index point at, so a recipe missing from it is a recipe with no inbound link:
+ * it renders, it is correct, and nobody arrives at it. Adding a page under
+ * {@code docs/recipes/} disturbs nothing when the catalogue is left alone —
+ * every gate stays green and the omission surfaces only when someone goes
+ * looking for a topic they were sure had been written up.
+ *
+ * The check runs the other way too. A catalogue row pointing at a file that
+ * was renamed or removed is a dead link on the most-linked documentation page,
+ * and nothing else in the build reads these paths.
+ */
+class RecipeCatalogueGuardTest {
+
+ private static final Path RECIPES_DIR = RepoRoot.get().resolve("docs/recipes");
+ private static final Path CATALOGUE = RepoRoot.get().resolve("docs/recipes.md");
+
+ /**
+ * {@code README.md} is what GitHub renders when the directory is opened. It
+ * points at the catalogue rather than being listed in it.
+ */
+ private static final String FOLDER_INDEX = "README.md";
+
+ /**
+ * A catalogue link, as written: {@code (recipes/.md)}, optionally with an
+ * anchor.
+ *
+ * The file name is anything a path segment can hold, not the lower-case-and-
+ * hyphens the current pages happen to use. Baking today's spelling in would make
+ * a correctly linked {@code pdf_export.md} report as unlisted — a false alarm
+ * rather than a missed page, but a guard that cries wolf over a legal file name
+ * is one the next person edits until it stops complaining.
+ */
+ private static final Pattern LINK = Pattern.compile("\\(recipes/([^/#)\\s]+\\.md)(?:#[^)]*)?\\)");
+
+ @Test
+ void everyRecipePageIsListedInTheCatalogue() throws IOException {
+ Set onDisk = recipeFiles();
+ Set linked = cataloguedFiles();
+
+ TreeSet unlisted = new TreeSet<>(onDisk);
+ unlisted.removeAll(linked);
+
+ assertThat(unlisted)
+ .describedAs("recipe pages under docs/recipes/ that docs/recipes.md never links — "
+ + "an unlinked page is one readers cannot arrive at")
+ .isEmpty();
+ }
+
+ @Test
+ void theCatalogueLinksNoRecipeThatIsGone() throws IOException {
+ Set onDisk = recipeFiles();
+ Set linked = cataloguedFiles();
+
+ TreeSet dangling = new TreeSet<>(linked);
+ dangling.removeAll(onDisk);
+
+ assertThat(dangling)
+ .describedAs("docs/recipes.md links files that no longer exist under docs/recipes/")
+ .isEmpty();
+ }
+
+ private static Set recipeFiles() throws IOException {
+ try (Stream files = Files.list(RECIPES_DIR)) {
+ Set names = files
+ .map(p -> p.getFileName().toString())
+ .filter(name -> name.endsWith(".md"))
+ .filter(name -> !FOLDER_INDEX.equals(name))
+ .collect(java.util.stream.Collectors.toCollection(TreeSet::new));
+
+ assertThat(names)
+ .describedAs("no recipe pages found under %s — the guard is reading a folder "
+ + "that moved and would cover nothing", RECIPES_DIR)
+ .isNotEmpty();
+ return names;
+ }
+ }
+
+ private static Set cataloguedFiles() throws IOException {
+ assertThat(CATALOGUE)
+ .describedAs("the recipe catalogue moved; this guard no longer reads the page it protects")
+ .exists();
+
+ List lines = Files.readAllLines(CATALOGUE, StandardCharsets.UTF_8);
+ TreeSet linked = new TreeSet<>();
+ for (String line : lines) {
+ Matcher link = LINK.matcher(line);
+ while (link.find()) {
+ linked.add(link.group(1));
+ }
+ }
+ return linked;
+ }
+}