feat(forge): a Testing section in the PR template, and the whole checklist in every PR #72

Merged
HumanBean merged 2 commits from HumanBean/Longshot:pr-testing-section into main 2026-11-13 01:44:02 +00:00
Owner

Why

A PR's "Not tested" line mixed up what was run, what wasn't and what someone else has to run, so testers couldn't see what was asked of them, and some PRs dropped the checklist altogether. This gives the template one Testing section in three parts, keeps the whole checklist in every PR, and has the check and pr ready enforce both.

What changed

  • template: .forgejo/pull_request_template.md's "Not tested" becomes Testing, in three labelled parts: Tested (what was run and checked, where and how, including what was checked but not measured), Not tested (what nobody has run yet, and why) and Needs other testers (one line each, with what to check, the result expected and its test request, or "request to open"; "None" when there is none). The checklist has a comment saying to keep it whole, and a new For everyone box: "Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests)".
  • check: scripts/check_pr_description.py requires Testing and its three parts, filled in with no placeholder left (testing_parts, testing_problems); each Needs other testers line needs a test request (a T-XXXX id or a link) or "request to open". It fails a ready PR with no ## Checklist, a missing or unticked For everyone box, or a deleted If applicable list. Warnings: Not tested or Needs other testers naming something the Measurements' Hardware line or table measured (measured), and a request still to open (requests_to_open).
  • scripts/forge.py: pr ready refuses while a test request is still "request to open"; pr status prints a Testing line (the parts, and how many testers' lines have a request or are still to open). In a second commit, pr create, fork and pr list --mine take the login from the server (me(), GET /user): the one saved at sign-in had gone stale after the account's rename, so pr create looked for the fork under the old name.
  • docs: CONTRIBUTING.md's description list (Testing in a few lines, the checklist kept whole, ready only once the test requests are open), docs/workflow.md's test step, docs/forge.md's pr ready.
  • tests: tests/test_check_pr_description.py (Testing and Checklist cases), tests/test_forge_pr.py (pr ready waiting for the requests, pr status's Testing line, the server's login), tests/pr_description/good.md with a Testing section.

Flags and settings

None

Measurements

main: d725b363 (2026-11-13)
this PR: pr-testing-section @ 2eff8fb1
Hardware: Ryzen desktop, CachyOS (Linux 7.3), Python 3.13
Scene: no game run: the game-free checks only
Method: LONGSHOT_VULKAN_HEADERS=/usr/include python -I -B tests/run_all.py, the whole run, two interleaved runs each (main, this PR, main, this PR); the two description tests by themselves on each side
How to reproduce: jj new main@forge or this branch, then the command above; python -I -B tests/test_check_pr_description.py; python -I -B tests/test_forge_pr.py

main @ d725b363 this PR @ 2eff8fb1 Δ
tests/run_all.py: passed (run 1 / run 2) 491 / 491 491 / 491 0 (no new test file)
tests/run_all.py: failed 2 / 2 2 / 2 0 (the same two)
tests/run_all.py: skipped 38 / 38 38 / 38 0
tests/run_all.py: time (run 1 / run 2) 101 / 87 s 86 / 88 s about the same
tests/test_check_pr_description.py 14 tests pass 18 tests pass +4
tests/test_forge_pr.py 18 tests pass 19 tests pass +1
main @ d725b363:
491 passed, 2 failed, 38 skipped in 101 s (linux, 24 at once)
491 passed, 2 failed, 38 skipped in 87 s (linux, 24 at once)
this PR @ 2eff8fb1:
491 passed, 2 failed, 38 skipped in 86 s (linux, 24 at once)
491 passed, 2 failed, 38 skipped in 88 s (linux, 24 at once)
both, each run:
FAIL 0.1s tests/test_bake_portability.py
FAIL 0.1s tests/test_builder_log_read_only.py

The two failures are the known ones here, on main as well (test_bake_portability: no zstandard; test_builder_log_read_only: no libtk).

Testing

Tested: on the Linux PC above: the whole game-free suite, twice on each side, and the two description tests; on git.magicbean.dev, this PR itself: opened with forge.py pr create (WIP, from the template with the commits filled in, from HumanBean's fork after the rename), filled in with pr update, then pr ready and pr status.

Not tested: Windows: no Windows PC here; the change is text handling in standard-library Python, with no path or OS code touched. The PR description workflow: Actions are off and no runner serves the repository yet.

Needs other testers: None: nothing here depends on the hardware or the OS.

Screenshots

Nothing on screen.

Risk and rollback

Open PRs written against the previous template (with "Not tested") fail the check once they are marked ready, until their description gets the Testing section; WIP ones only get the notice. Reverting the commits brings back the previous template and check.

Reviewer notes

Start with testing_problems and the checklist part of review() in scripts/check_pr_description.py, then complete_or_refuse in scripts/forge.py. "request to open" passes the check itself, with a warning, and pr ready refuses it, so a WIP PR can name a request it hasn't opened yet. The second commit (me()) is a small separate fix that opening this PR needed.

Issues

See #48

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers. n/a
  • Flags table filled in; README / docs/development.md switch reference updated. n/a
  • Screenshots for a change that shows on screen. n/a
  • A row in docs/tested-configurations.md for each measured run. n/a
  • MKWVK_API_VERSION bumped (gpu/vk/vk_api.h's MkwVkHost or MkwVkRenderer changed). n/a
  • The SPIR-V headers regenerated with glslang (a shader changed). n/a
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened. n/a
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. n/a
  • Docs updated.
## Why A PR's "Not tested" line mixed up what was run, what wasn't and what someone else has to run, so testers couldn't see what was asked of them, and some PRs dropped the checklist altogether. This gives the template one Testing section in three parts, keeps the whole checklist in every PR, and has the check and `pr ready` enforce both. ## What changed - template: `.forgejo/pull_request_template.md`'s "Not tested" becomes **Testing**, in three labelled parts: **Tested** (what was run and checked, where and how, including what was checked but not measured), **Not tested** (what nobody has run yet, and why) and **Needs other testers** (one line each, with what to check, the result expected and its test request, or "request to open"; "None" when there is none). The checklist has a comment saying to keep it whole, and a new *For everyone* box: "Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests)". - check: `scripts/check_pr_description.py` requires Testing and its three parts, filled in with no placeholder left (`testing_parts`, `testing_problems`); each Needs other testers line needs a test request (a `T-XXXX` id or a link) or "request to open". It fails a ready PR with no `## Checklist`, a missing or unticked *For everyone* box, or a deleted *If applicable* list. Warnings: Not tested or Needs other testers naming something the Measurements' Hardware line or table measured (`measured`), and a request still to open (`requests_to_open`). - `scripts/forge.py`: `pr ready` refuses while a test request is still "request to open"; `pr status` prints a Testing line (the parts, and how many testers' lines have a request or are still to open). In a second commit, `pr create`, `fork` and `pr list --mine` take the login from the server (`me()`, `GET /user`): the one saved at sign-in had gone stale after the account's rename, so `pr create` looked for the fork under the old name. - docs: CONTRIBUTING.md's description list (Testing in a few lines, the checklist kept whole, ready only once the test requests are open), `docs/workflow.md`'s test step, `docs/forge.md`'s `pr ready`. - tests: `tests/test_check_pr_description.py` (Testing and Checklist cases), `tests/test_forge_pr.py` (`pr ready` waiting for the requests, `pr status`'s Testing line, the server's login), `tests/pr_description/good.md` with a Testing section. ## Flags and settings None ## Measurements main: d725b363 (2026-11-13) this PR: pr-testing-section @ 2eff8fb1 Hardware: Ryzen desktop, CachyOS (Linux 7.3), Python 3.13 Scene: no game run: the game-free checks only Method: `LONGSHOT_VULKAN_HEADERS=/usr/include python -I -B tests/run_all.py`, the whole run, two interleaved runs each (main, this PR, main, this PR); the two description tests by themselves on each side How to reproduce: `jj new main@forge` or this branch, then the command above; `python -I -B tests/test_check_pr_description.py`; `python -I -B tests/test_forge_pr.py` | | main @ d725b363 | this PR @ 2eff8fb1 | Δ | |---|---|---|---| | tests/run_all.py: passed (run 1 / run 2) | 491 / 491 | 491 / 491 | 0 (no new test file) | | tests/run_all.py: failed | 2 / 2 | 2 / 2 | 0 (the same two) | | tests/run_all.py: skipped | 38 / 38 | 38 / 38 | 0 | | tests/run_all.py: time (run 1 / run 2) | 101 / 87 s | 86 / 88 s | about the same | | tests/test_check_pr_description.py | 14 tests pass | 18 tests pass | +4 | | tests/test_forge_pr.py | 18 tests pass | 19 tests pass | +1 | ``` main @ d725b363: 491 passed, 2 failed, 38 skipped in 101 s (linux, 24 at once) 491 passed, 2 failed, 38 skipped in 87 s (linux, 24 at once) this PR @ 2eff8fb1: 491 passed, 2 failed, 38 skipped in 86 s (linux, 24 at once) 491 passed, 2 failed, 38 skipped in 88 s (linux, 24 at once) both, each run: FAIL 0.1s tests/test_bake_portability.py FAIL 0.1s tests/test_builder_log_read_only.py ``` The two failures are the known ones here, on main as well (`test_bake_portability`: no zstandard; `test_builder_log_read_only`: no libtk). ## Testing **Tested:** on the Linux PC above: the whole game-free suite, twice on each side, and the two description tests; on git.magicbean.dev, this PR itself: opened with `forge.py pr create` (WIP, from the template with the commits filled in, from HumanBean's fork after the rename), filled in with `pr update`, then `pr ready` and `pr status`. **Not tested:** Windows: no Windows PC here; the change is text handling in standard-library Python, with no path or OS code touched. The `PR description` workflow: Actions are off and no runner serves the repository yet. **Needs other testers:** None: nothing here depends on the hardware or the OS. ## Screenshots Nothing on screen. ## Risk and rollback Open PRs written against the previous template (with "Not tested") fail the check once they are marked ready, until their description gets the Testing section; WIP ones only get the notice. Reverting the commits brings back the previous template and check. ## Reviewer notes Start with `testing_problems` and the checklist part of `review()` in `scripts/check_pr_description.py`, then `complete_or_refuse` in `scripts/forge.py`. "request to open" passes the check itself, with a warning, and `pr ready` refuses it, so a WIP PR can name a request it hasn't opened yet. The second commit (`me()`) is a small separate fix that opening this PR needed. ## Issues See #48 ## Checklist <!-- Keep the whole checklist, both lists, in every PR: tick what is done; in If applicable, leave unticked (or write "n/a" beside) what doesn't apply. --> ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [ ] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. n/a - [ ] Flags table filled in; README / docs/development.md switch reference updated. n/a - [ ] Screenshots for a change that shows on screen. n/a - [ ] A row in `docs/tested-configurations.md` for each measured run. n/a - [ ] `MKWVK_API_VERSION` bumped (`gpu/vk/vk_api.h`'s `MkwVkHost` or `MkwVkRenderer` changed). n/a - [ ] The SPIR-V headers regenerated with glslang (a shader changed). n/a - [x] A game-free test added for the fix, failing on `main` and passing here. - [ ] A check under `tests/` that reads changed source updated in this PR, not weakened. n/a - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. n/a - [x] Docs updated.
The template's Not tested section becomes Testing, in three parts: Tested (what was run and checked, where and how),
Not tested (what nobody has run yet, and why) and Needs other testers (a line each, with its test request, or
'request to open'; None when there is none). The checklist is kept whole in every PR, with a For everyone box for
Testing. check_pr_description.py requires both; naming as untested something the Measurements measured is a warning,
and so is a test request still to open, which forge.py pr ready refuses. pr status shows the Testing parts.
The login saved at sign-in goes stale when the account is renamed: pr create then looked for the fork under the old
name. It now asks the server (GET /user).
Author
Owner

Filled in the description: the Testing section and the checklist rules, the second commit's fix for the stale login after the rename, and the test runs on main @ d725b363 and this PR @ 2eff8fb1, two interleaved runs each (491 passed and the same 2 known failures on both; the description tests go from 14 to 18 and from 18 to 19).

Filled in the description: the Testing section and the checklist rules, the second commit's fix for the stale login after the rename, and the test runs on main @ d725b363 and this PR @ 2eff8fb1, two interleaved runs each (491 passed and the same 2 known failures on both; the description tests go from 14 to 18 and from 18 to 19).
HumanBean changed title from WIP: feat(forge): a Testing section in the PR template, and the whole checklist in every PR to feat(forge): a Testing section in the PR template, and the whole checklist in every PR 2026-11-13 01:40:19 +00:00
HumanBean deleted branch pr-testing-section 2026-11-13 01:44:02 +00:00
Sign in to join this conversation.
No description provided.