24 of 25 test verbs never run in CI, so most of this repo's tests only rot #969

New issue

Open

opened 2026-08-27 10:34:52 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-27 10:34:52 +00:00

Owner

Found while adding two test suites for the mirror watch (#967, #968) and checking where they would run. They would not.

The numbers

$ grep -cE "^[a-z0-9-]+-test \*ARGS:" justfile
25

$ grep -rhoE "just [a-z0-9-]+" .forgejo/workflows/ | sort -u
just ansible-sync-test
just pre-commit

ansible-sync-test is the only test verb any workflow invokes, and just pre-commit runs linters rather than tests. So 24 suites exist, pass when someone remembers to run them locally, and are never executed by anything on a push, a pull request, or a schedule.

That includes agent-git-attribution-test, which guards the shim that rewrites the author of every agent commit in the estate.

Why it matters more than the count suggests

A test nobody runs is worse than no test. It reads as coverage in review, so a reviewer relaxes where they otherwise would not, and it silently stops reflecting the code the first time either drifts. The failure is invisible by construction: nothing goes red, because nothing runs.

Two of these were written today with the deliberate property that they need no network and no credential, precisely so they could run anywhere. Nothing runs them.

What the work is

The verbs are not uniform, so this needs triage rather than one loop over all 25:

Hermetic and cheap. mirror-sync-check-test, mirror-sync-push-test, telegram-alert-test, and others need no network and no credential. These should run on every pull request, and there is no reason they do not.
Platform-gated. The windows-*-convergence-test family skips off-Windows. Running them in CI costs a skip line and catches an import error or syntax break, which is worth having.
Credentialed or networked. forgejo-branch-protection-test and github-app-token-test may need a token or a live forge. These want a decision: gate them behind a schedule, give CI a scoped credential, or state that they are operator-run and mark them so.

The obvious shape is one aggregate verb, something like just test, that runs the hermetic set, with the other two categories explicitly named rather than silently absent. Then lint.yml calls the aggregate rather than naming a single suite, so a new suite is covered by existing when it is written instead of needing a workflow edit nobody remembers.

Deliberately not doing this in the mirror-watch PRs

Adding only mirror-sync-check-test and mirror-sync-push-test to lint.yml would have made those two the second and third covered suites and left the other 23 exactly as they are, while making the gap look addressed. The estate-wide answer is worth more than special-casing whatever was written most recently.

agent-git-attribution-test additionally fails when run from inside an agent session, because the harness pre-exports AGENT_GIT_ATTRIBUTION_COAUTHOR and puts the real shim on PATH, so the test's staged shim resolves back to the ambient one and recurses until git exits 2. It passes in 0.38s with a sanitised environment. That is a separate defect in the test's isolation, and it would surface immediately once the suite runs in CI, where the ambient shim is absent.

Found while adding two test suites for the mirror watch (#967, #968) and checking where they would run. They would not. ## The numbers ``` $ grep -cE "^[a-z0-9-]+-test \*ARGS:" justfile 25 $ grep -rhoE "just [a-z0-9-]+" .forgejo/workflows/ | sort -u just ansible-sync-test just pre-commit ``` `ansible-sync-test` is the only test verb any workflow invokes, and `just pre-commit` runs linters rather than tests. So 24 suites exist, pass when someone remembers to run them locally, and are never executed by anything on a push, a pull request, or a schedule. That includes `agent-git-attribution-test`, which guards the shim that rewrites the author of every agent commit in the estate. ## Why it matters more than the count suggests A test nobody runs is worse than no test. It reads as coverage in review, so a reviewer relaxes where they otherwise would not, and it silently stops reflecting the code the first time either drifts. The failure is invisible by construction: nothing goes red, because nothing runs. Two of these were written today with the deliberate property that they need no network and no credential, precisely so they could run anywhere. Nothing runs them. ## What the work is The verbs are not uniform, so this needs triage rather than one loop over all 25: * **Hermetic and cheap.** `mirror-sync-check-test`, `mirror-sync-push-test`, `telegram-alert-test`, and others need no network and no credential. These should run on every pull request, and there is no reason they do not. * **Platform-gated.** The `windows-*-convergence-test` family skips off-Windows. Running them in CI costs a skip line and catches an import error or syntax break, which is worth having. * **Credentialed or networked.** `forgejo-branch-protection-test` and `github-app-token-test` may need a token or a live forge. These want a decision: gate them behind a schedule, give CI a scoped credential, or state that they are operator-run and mark them so. The obvious shape is one aggregate verb, something like `just test`, that runs the hermetic set, with the other two categories explicitly named rather than silently absent. Then `lint.yml` calls the aggregate rather than naming a single suite, so a new suite is covered by existing when it is written instead of needing a workflow edit nobody remembers. ## Deliberately not doing this in the mirror-watch PRs Adding only `mirror-sync-check-test` and `mirror-sync-push-test` to `lint.yml` would have made those two the second and third covered suites and left the other 23 exactly as they are, while making the gap look addressed. The estate-wide answer is worth more than special-casing whatever was written most recently. ## Related `agent-git-attribution-test` additionally fails when run from inside an agent session, because the harness pre-exports `AGENT_GIT_ATTRIBUTION_COAUTHOR` and puts the real shim on `PATH`, so the test's staged shim resolves back to the ambient one and recurses until git exits 2. It passes in 0.38s with a sanitised environment. That is a separate defect in the test's isolation, and it would surface immediately once the suite runs in CI, where the ambient shim is absent.

coilyco-ops added this to the CI and repo hygiene milestone

2026-08-29 06:10:27 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

coilyco-flight-deck/infrastructure#969

No description provided.

Rows
Columns