Skip to content

test(ci): keep the docs/CI.md check table aligned with workflow jobs - #274

Open
raymondginger2018-sudo wants to merge 2 commits into
HKUDS:mainfrom
raymondginger2018-sudo:pr/tests-ci-doc-table
Open

raymondginger2018-sudo wants to merge 2 commits into
HKUDS:mainfrom
raymondginger2018-sudo:pr/tests-ci-doc-table

Conversation

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor

Summary

docs/CI.md explains each CI check in a table, but nothing stops the table from drifting when a workflow gains, loses, or renames a job. This PR adds a small regression test that parses every workflow file and requires each job to be either mapped to the phrase that documents it in docs/CI.md or explicitly allowlisted as release-only.

Changes

  • tests/test_ci_documentation.py (new file only):
    • test_every_workflow_job_is_classified — parses .github/workflows/*.{yml,yaml} with pyyaml (already pinned in requirements.lock; test_ci_installation.py is a precedent for import yaml in tests) and asserts the file set and each file's job set match CHECK_TABLE | RELEASE_ONLY_JOBS exactly: a new workflow file, a new job, or a renamed job fails until classified or allowlisted.
    • test_check_table_phrases_still_appear_in_the_docs — asserts every mapped phrase still exists in docs/CI.md (case-insensitive), so deleting a documented row fails too.
    • RELEASE_ONLY_JOBS allowlists the three release-only workflows (deploy-website.yml, pypi-publish.yml, release-desktop.yml) — jobs that never gate a pull request.
    • Reads are explicitly UTF-8 (encoding="utf-8") so the test does not depend on the runner locale (a plain read_text() dies on Windows cp936 when the docs contain an en-dash).

Tests

  • pytest -q tests/test_ci_documentation.py → 2 passed (Python 3.12.14, pyyaml 6.0.3).
  • ruff check and ruff format --check with the CI-pinned ruff 0.15.21 → clean.
  • Local preflight: KNOWN-RED — verdict line PREFLIGHT: KNOWN-RED | jobs=8 | rc0=14 | new=0 | known=1 | sanitize=clean | tree_head=12c9862e (new=0: no new reds; the single known red is the baseline job-windows-lifecycle:group2_rc=1, expected on local Windows). notrun=desktop,bundle,web-browser matches upstream PR scope gating for a tests-only change.

Dependency note

  • Standalone: all ten mapped phrases already exist in docs/CI.md on main, so the test passes without any docs change; independent of the docs reproduction PR (no file overlap).

AI-Assisted Disclosure

  1. I have read every line and can explain the classification of all 16 jobs across the 8 workflow files.
  2. Locally verified per the tests above plus the local preflight gate.
  3. Single topic: docs/CI.md ↔ workflow inventory drift detection only.
  4. AI tool: DeepSeek Harness Agent assisted drafting; final review by me.

@Zongwei9888

Copy link
Copy Markdown
Collaborator

Thank you, @raymondginger2018-sudo. The job-inventory test is useful, and the first test is good as is. Three changes before merge:

  1. Match the table, not the whole file. phrase in ci_doc searches the whole document, so lint-and-format passes on "formatting" from the intro (line 7) and from the Desktop row's "Rust formatting"; the table has no lint row today, and the test can't see that. Please read the rows of the "Check | What it verifies" table and match against the first column only.
  2. Add a lint row to the table (pre-commit, run on every push and PR by linting.yaml), so lint-and-format maps to a row of its own.
  3. Fix the comment. It says jobs that never gate a pull request stay out of the table, but dependency-audit and bundle run only on schedule or manual dispatch and are in CHECK_TABLE. Something like "release-only jobs stay out of the table" would match the code.

Note that #273 is now on main, so please rebase first.

Review fixes: parse the Check table's first column instead of searching
the whole doc (intro/Rust-formatting false passes), give lint-and-format
its own table row, and reword the allowlist comment to release-only so
it matches dependency-audit/bundle living in CHECK_TABLE.
@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor Author

@Zongwei9888 — all three changes are pushed as 5efea265:

  1. Table-only matching. test_ci_documentation.py now parses the Check | What it verifies table and matches CHECK_TABLE phrases against the first column only (_check_table_first_columns()), so the intro's "formatting" and the Desktop row's "Rust formatting" no longer count.
  2. Lint row added. docs/CI.md now has | Formatting and lint (lint-and-format) | pre-commit, run on every push and PR by linting.yaml |, so lint-and-format maps to a row of its own.
  3. Comment reworded. The allowlist comment now reads "Release-only jobs stay out of the check table.", matching dependency-audit/bundle living in CHECK_TABLE.

On the rebase: the branch is already based on the current main tip (8332d745, which includes the #273 merge) — the two commits were rebuilt on top of it when the review fixes were written, so no additional rebase was needed.

Local preflight of this exact head (Windows, Python 3.12/3.13/3.14): no new failures; the only red is the already-tracked Windows-only lifecycle flake.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants