-
Notifications
You must be signed in to change notification settings - Fork 243
docs: check documented flags against the binary in CI #4041
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| permissions: {} | ||
| name: Docs Flag Parity | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| paths: | ||
| - "docs/**" | ||
| schedule: | ||
| - cron: "0 8 * * 1" | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| flag-parity: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| # Full history: the published docs are checked against their release tag. | ||
| fetch-depth: 0 | ||
| - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6.0.0 | ||
| with: | ||
| node-version-file: docs/.nvmrc | ||
|
|
||
| - name: Check docs flags against the binary | ||
| run: node docs/check-flags.js | ||
|
|
||
| - name: Alert Slack on drift | ||
| if: failure() && github.event_name != 'pull_request' | ||
| env: | ||
| SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
| run: | | ||
| MESSAGE="🚨 *Juno docs drift:* flags on juno.nethermind.io no longer match the binary.\n\nA flag was likely renamed or added in \`cmd/juno/juno.go\` after the docs were written. Run \`node docs/check-flags.js\` for the list.\n\n- *Run:* <https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}|workflow log>" | ||
| curl -X POST -H 'Content-type: application/json' --data "{\"text\":\"$MESSAGE\"}" "$SLACK_WEBHOOK_URL" |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems awfully complicated and unnecessary. Why does this do? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| #!/usr/bin/env node | ||
| // Verifies the docs and the binary agree about flags, in both directions. Next docs | ||
| // are checked against the working tree; published docs against their line's newest stable tag. | ||
|
|
||
| const fs = require("fs"); | ||
| const path = require("path"); | ||
| const { execFileSync } = require("child_process"); | ||
| const { | ||
| preprocessCodebase, | ||
| extractConfigs, | ||
| generateConfigTable, | ||
| } = require("./generate-config.js"); | ||
|
|
||
| const repoRoot = path.resolve(__dirname, ".."); | ||
| // Registered by cobra itself, so real flags without a Flags() call in juno.go. | ||
| const COBRA_BUILTINS = new Set(["help", "version"]); | ||
|
|
||
| const findings = []; | ||
| const fail = (file, msg) => | ||
| findings.push(`${path.relative(repoRoot, file)}: ${msg}`); | ||
|
|
||
| function flagNames(goSource) { | ||
| const names = extractConfigs(preprocessCodebase(goSource)) | ||
| .map((c) => c.configName) | ||
| .filter((n) => typeof n === "string"); | ||
| return new Set([...names, ...COBRA_BUILTINS]); | ||
| } | ||
|
|
||
| // Flags a fenced line passes to Juno. Docker's own flags sit before the image, | ||
| // and helm charts are also named nethermind/juno, so the image rule is docker-only. | ||
| function flagsFromCommand(line) { | ||
| // Env-var prefixes (`JUNO_HTTP=true juno ...`) would otherwise hide the command. | ||
| let args = line.trim().replace(/^(?:[A-Z_][A-Z0-9_]*=\S+\s+)+/, ""); | ||
| // Anchored so nethermind/juno-plugin and friends do not match. | ||
| const image = | ||
| /\bdocker\s+(run|create)\b/.test(args) && | ||
| args.match(/nethermind\/juno(?::\S+|@\S+)?(?=\s|$)/); | ||
| if (image) { | ||
| args = args.slice(image.index + image[0].length); | ||
| } else if (/^(\S*\/)?juno\s/.test(args)) { | ||
| args = args.replace(/^(\S*\/)?juno\s+/, ""); | ||
| } else { | ||
| return []; | ||
| } | ||
| return [...args.matchAll(/(?:^|\s)--([a-z][a-z0-9-]*)/g)].map((m) => m[1]); | ||
| } | ||
|
|
||
| // Every --flag a page mentions: backticked in prose, or passed to Juno in a fence. | ||
| function flagMentions(markdown) { | ||
| const mentions = new Map(); // name -> first line, for the report | ||
| const lines = markdown.split("\n"); | ||
| let fenceMarker = null; // a fence only closes on its own marker | ||
| let joined = ""; | ||
| for (let i = 0; i < lines.length; i++) { | ||
| const l = lines[i]; | ||
| const fence = l.trim().match(/^(```|~~~)/); | ||
| if (fence && (!fenceMarker || fence[1] === fenceMarker)) { | ||
| fenceMarker = fenceMarker ? null : fence[1]; | ||
| joined = ""; | ||
| continue; | ||
| } | ||
| const inFence = fenceMarker !== null; | ||
| if (inFence) { | ||
| joined += l.endsWith("\\") ? l.slice(0, -1) + " " : l; | ||
| if (l.endsWith("\\")) continue; | ||
| for (const f of flagsFromCommand(joined)) { | ||
| if (!mentions.has(f)) mentions.set(f, i + 1); | ||
| } | ||
| joined = ""; | ||
| } else { | ||
| // A removed or renamed flag must stay documentable, so those lines are exempt. | ||
| if (/\b(removed|renamed|deprecated)\b/i.test(l)) continue; | ||
| for (const m of l.matchAll(/`--([a-z][a-z0-9-]*)[^`]*`/g)) { | ||
| if (!mentions.has(m[1])) mentions.set(m[1], i + 1); | ||
| } | ||
| } | ||
| } | ||
| return mentions; | ||
| } | ||
|
|
||
| // First cell of each row in a generated config table. | ||
| function tableNames(markdown) { | ||
| const names = new Map(); | ||
| markdown.split("\n").forEach((l, i) => { | ||
| const m = l.match(/^\| `([a-z0-9-]+)` \|/); | ||
| if (m && !names.has(m[1])) names.set(m[1], i + 1); | ||
| }); | ||
| return names; | ||
| } | ||
|
|
||
| function checkTree(treeDir, source) { | ||
| const names = flagNames(source.text); | ||
|
|
||
| // Documented flags must exist in the binary: the --log-port class. | ||
| for (const file of fs.readdirSync(treeDir).filter((f) => f.endsWith(".md"))) { | ||
| const p = path.join(treeDir, file); | ||
| const md = fs.readFileSync(p, "utf8"); | ||
| const documented = file.startsWith("_") ? tableNames(md) : flagMentions(md); | ||
| for (const [flag, line] of documented) { | ||
| if (!names.has(flag)) { | ||
| fail(p, `line ${line}: \`--${flag}\` is not a flag in ${source.label}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Registered flags must be in the table: the undocumented-flag class. | ||
| const tableFile = path.join(treeDir, "_config-options.md"); | ||
| if (!fs.existsSync(tableFile)) { | ||
| fail(tableFile, "config table missing from this tree"); | ||
| return; | ||
| } | ||
| const inTable = tableNames(fs.readFileSync(tableFile, "utf8")); | ||
| for (const name of names) { | ||
| if (COBRA_BUILTINS.has(name)) continue; | ||
| if (!inTable.has(name)) { | ||
| fail(tableFile, `missing \`${name}\`, which ${source.label} registers`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function main() { | ||
| // next: the working tree is the truth. | ||
| const localSource = fs.readFileSync( | ||
| path.join(repoRoot, "cmd", "juno", "juno.go"), | ||
| "utf8", | ||
| ); | ||
| checkTree(path.join(__dirname, "docs"), { | ||
| label: "cmd/juno/juno.go (working tree)", | ||
| text: localSource, | ||
| }); | ||
|
|
||
| // next only: the committed table must byte-match the generator's output. | ||
| const expected = generateConfigTable( | ||
| extractConfigs(preprocessCodebase(localSource)), | ||
| ); | ||
| const tablePath = path.join(__dirname, "docs", "_config-options.md"); | ||
| if (fs.readFileSync(tablePath, "utf8") !== expected) { | ||
| fail( | ||
| tablePath, | ||
| "stale against cmd/juno/juno.go (working tree); run `cd docs && node generate-config.js` and commit the result", | ||
| ); | ||
| } | ||
|
|
||
| // published: checked against the newest stable tag of its line. | ||
| const published = JSON.parse( | ||
| fs.readFileSync(path.join(__dirname, "versions.json"), "utf8"), | ||
| )[0]; | ||
| const line = published.split(".").slice(0, 2).join("."); | ||
| const tags = execFileSync("git", ["tag", "-l", `v${line}.*`], { | ||
| cwd: repoRoot, | ||
| }) | ||
| .toString() | ||
| .split("\n") | ||
| .filter((t) => /^v\d+\.\d+\.\d+$/.test(t)) // stable releases only | ||
| .sort((a, b) => Number(a.split(".")[2]) - Number(b.split(".")[2])); | ||
| const tag = tags[tags.length - 1]; | ||
| if (!tag) { | ||
| // Shallow clone without tags: warn instead of failing; CI fetches full history. | ||
| console.warn(`no v${line}.* tag found; skipping the published-version check`); | ||
| } else { | ||
| const tagSource = execFileSync( | ||
| "git", | ||
| ["show", `${tag}:cmd/juno/juno.go`], | ||
| { cwd: repoRoot }, | ||
| ).toString(); | ||
| checkTree(path.join(__dirname, "versioned_docs", `version-${published}`), { | ||
| label: `${tag} (newest stable of the ${line} line)`, | ||
| text: tagSource, | ||
| }); | ||
| } | ||
|
|
||
| if (findings.length) { | ||
| console.error(`${findings.length} finding(s):\n`); | ||
| for (const f of findings) console.error(` FAIL ${f}`); | ||
| process.exit(1); | ||
| } | ||
| console.log( | ||
| `docs and binary agree on flags (next vs cmd/juno/juno.go, ${published} vs ${tag || "skipped"})`, | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| main(); | ||
| } catch (err) { | ||
| // Environment problems (no git, broken tree) are not docs findings. | ||
| console.error(`check-flags: ${err.message}`); | ||
| process.exit(2); | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice catch |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,14 +203,18 @@ In case you want to change the log level in runtime without the need to restart | |
|
|
||
| To enable this feature, use the following configuration options: | ||
|
|
||
| - `log-host`: The interface to listen for requests. Defaults to `localhost`. | ||
| - `log-port`: The port to listen for requests. REQUIRED | ||
| :::note | ||
| These flags were renamed from `--log-host` and `--log-port` in v0.14.4. When running in a container, set `--http-update-host=0.0.0.0`; the default binds localhost. | ||
| ::: | ||
|
|
||
|
Comment on lines
+206
to
+209
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove |
||
| - `http-update-host`: The interface to listen for requests. Defaults to `localhost`. | ||
| - `http-update-port`: The port to listen for requests. REQUIRED | ||
|
|
||
| Examples: | ||
|
|
||
| ```console | ||
| # Start juno specifying the log port | ||
| juno --log-port=6789 --log-level=error ... | ||
| # Start juno specifying the http-update port | ||
| juno --http-update-port=6789 --log-level=error ... | ||
|
|
||
| # Get current level | ||
| curl -X GET 'localhost:6789/log/level' | ||
|
|
||
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.
I think it is better to add the job to docs-test.yml