-
Notifications
You must be signed in to change notification settings - Fork 1
ci(make): run the docs-integrity gate inside make ci #403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
bketelsen
wants to merge
1
commit into
main
Choose a base branch
from
ci/docs-integrity-in-ci
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+82
−5
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package updex | ||
|
|
||
| import ( | ||
| "os" | ||
| "regexp" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| // docsGateInvocations are the two commands the `docs-gate` job in | ||
| // .github/workflows/test.yml runs. `make ci` claims to be the credential-free | ||
| // local equivalent of that workflow, so it must run both as well. | ||
| var docsGateInvocations = []struct { | ||
| name string | ||
| pattern *regexp.Regexp | ||
| }{ | ||
| { | ||
| name: "make test-docs-check", | ||
| pattern: regexp.MustCompile(`(?m)^\t@?(?:\$\(MAKE\)|make)(?:\s+--no-print-directory)?\s+test-docs-check\s*$`), | ||
| }, | ||
| { | ||
| name: "node scripts/check-docs.mjs", | ||
| pattern: regexp.MustCompile(`(?m)^\t@?node\s+scripts/check-docs\.mjs\s*$`), | ||
| }, | ||
| } | ||
|
|
||
| // TestMakefileCIRecipeRunsDocsIntegrityGate pins the docs-integrity checks | ||
| // into `make ci`. GitHub CI runs `make test-docs-check` and | ||
| // `node scripts/check-docs.mjs` in its `docs-gate` job; before this guard the | ||
| // local `ci` recipe ran neither, so a broken docs index, a dead relative link, | ||
| // or a broken conformance alias passed the local gate and only failed after a | ||
| // pull request was already open. The test fails if either invocation is | ||
| // dropped from the recipe, is moved after the final "CI gate passed" line, or | ||
| // has its failure swallowed. | ||
| func TestMakefileCIRecipeRunsDocsIntegrityGate(t *testing.T) { | ||
| data, err := os.ReadFile("../Makefile") | ||
| if err != nil { | ||
| t.Fatalf("read Makefile: %v", err) | ||
| } | ||
|
|
||
| recipe := extractRecipe(t, string(data), "ci:") | ||
|
|
||
| passIdx := strings.Index(recipe, "CI gate passed") | ||
| if passIdx < 0 { | ||
| t.Fatalf("Makefile ci recipe no longer prints \"CI gate passed\"; got:\n%s", recipe) | ||
| } | ||
|
|
||
| for _, invocation := range docsGateInvocations { | ||
| loc := invocation.pattern.FindStringIndex(recipe) | ||
| if loc == nil { | ||
| t.Errorf("Makefile ci recipe must invoke `%s` (the docs-gate CI job runs it); got:\n%s", invocation.name, recipe) | ||
| continue | ||
| } | ||
| if loc[0] > passIdx { | ||
| t.Errorf("Makefile ci recipe invokes `%s` after printing \"CI gate passed\"; move it before the success line", invocation.name) | ||
| } | ||
| } | ||
|
|
||
| for _, line := range strings.Split(recipe, "\n") { | ||
| trimmed := strings.TrimPrefix(line, "\t") | ||
| if !strings.Contains(trimmed, "test-docs-check") && !strings.Contains(trimmed, "check-docs.mjs") { | ||
| continue | ||
| } | ||
| if strings.HasPrefix(trimmed, "-") || strings.HasPrefix(trimmed, "@-") { | ||
| t.Errorf("Makefile ci recipe ignores the exit status of a docs-integrity invocation: %q", line) | ||
| } | ||
| if strings.Contains(trimmed, "||") { | ||
| t.Errorf("Makefile ci recipe swallows a docs-integrity failure with a `||` fallback: %q", line) | ||
| } | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed, and the finding is correct on both counts at head 5ac9b99:
docs/design/overview.md"CI and Releases" (lines 583-592) enumerates themake cistages as verify-static -> coverage -> E2E -> race -> cross-build.grep -n 'docs-integrity\|check-docs' docs/design/overview.mdreturns nothing, so the file never mentions the two steps this PR inserts.docs/specs/pr-review-rubric.md:24("Build gate green") likewise enumeratesmake cias tidy, vet, gofmt, golangci-lint, non-E2E unit tests, coverage gate, race, cross-build. The separate "Docs-integrity gate green" row namesnode scripts/check-docs.mjsbut not as part ofmake ci, so the enumeration is now incomplete.This is a patch change, not a mechanical cure, so it cannot be pushed under the
pr-cureitem that surfaced this thread (Snowcat refuses a cure whose patch identity moved). I have queued a boundedpr-cure-changeproposal against this same branch to update both passages to match theci:recipe in the Makefile — nothing else in either file. Leaving this thread unresolved until that lands.