WIP: feat(test): pin test requests and results to commits #71
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
needs measurements
question
testing wanted
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!71
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:feat/test-requests-pin-commit"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
A test result must identify the commit tested, rather than silently appearing to cover a later version of the pull request. This is a new adaptation of https://github.com/NotRexed/Hookshot/issues/2830 to Longshot's Forgejo test-request client, not a replay of its GitHub implementation. It remains a draft until signed-in room posting and the Discord copy are verified.
What changed
Commit:/Head:footers retain the pin and observed head. Earlier metadata-looking note lines stay notes. Claims keep their own commit/head snapshots; release, expiry, done and cancel clear them.--commitconfirms the same existing pin; it cannot substitute a different one. Only the requester may update an open card.resultsremain available.current_resultsfilters by the card's pin and drives JSON, ordering, display and already-tested refusals.donerecords the claimed commit, refusing a different one. After expiry/release it requires an explicit full tested SHA. ItsHead:is the saved claim/request observation, not a fresh reporting-time lookup.--commitoptions refuse rather than silently doing nothing.verifycommand. The old GitHub full-SHA outage fallback is not carried: new request/update pin validation needs a successful Forgejo lookup.The main client was checked again at this move's start. Its latest change is
8f7d0d37b, the Forgejo-specific client migration; the source at current maind4a69456is byte-identical to the earlier adaptation base's client. The adaptation therefore includes today's forge tool, rather than replacing it with the old GitHub script.Flags and settings
request --commitclaim --commitdone --commitupdate --commitclaim --againlist --jsonshow --jsonrequest --notesMeasurements
Branch state: on main
d725b3631982ed0035ea3ac9cbe1677e9fad260c; heade673c7dddb144cb0bbad75aeae7342299c6db419; whole game-free suite on this head:510 passed, 2 failed, 20 skipped(maind725b3631982ed0035ea3ac9cbe1677e9fad260c:510 passed, 1 failed, 20 skipped); the commits are unchanged by rebases apart from the results table.Known local failure in the measured suites and both suites in the branch-state block:
tests/test_updater_asset_digest.py; local SDK 10.0.113 cannot resolveMicrosoft.AspNetCore.App.Ref (= 10.0.13), errorNU1100. This was the measured branch's only failure.The refreshed branch suite additionally retained
tests/test_display_pointer.py:FAIL line 509: reads > 0. The identical-input fixture then passed alone on both main and this branch. This is a timing-dependent observation under shared load, not a persistent baseline failure or an erased suite failure; the complete new main suite did not reproduce it. This PR remains WIP, with the suite box unticked as well as the already disclosed room/Discord integration gap.main:
d4a69456c9(2026-11-12)this PR: feat/test-requests-pin-commit @ 938ab23317bb301f9d0120c5a8d6d0f7af113c13
Measurement provenance: main @
d4a69456c9. 938ab23317bb301f9d0120c5a8d6d0f7af113c13, including final documentation/evidence rows.Hardware: Core i9-13900K, 96 GB RAM, Arch Linux 7.2.15, pinned Python 3.12. No GPU or game needed.
Scene: offline request, moved-head claim and done cases using the actual main/branch client functions; authentication, room and HTTP operations are mocked and unexpected live calls fail closed.
Method: both complete queue test programs as warm-up, then the same three cases before/after/before/after, fresh HOME each time. Median passing-case count is reported, not execution speed. Full suite afterward on the exact final commit. Three separate unauthenticated public GETs check the current commit/comparison response shape; they are not a room trial.
How to reproduce: Run
python -I -B tests/run_all.py test_test_requests. For the same paired negatives, load the three named PinTests cases (request_footer_and_note_lines_round_trip, claim_default_refuses_head_drift_explicit_confirms_same_pin, done_defaults_claimed_pin_and_clears_snapshots) with their production module bound to main and then this PR, retaining the fixtures' fail-closed mocks; repeat in that order. Do not invoke the live room commands for this offline check.Main
d725b3631adds only scripts, docs and tests, with no game-code change; the measuredd4a69456comparisons and pinned game evidence remain valid and were not repeated. Original results below retain their measured commits; the new suite is recorded in the branch-state block.Baseline: main @
d4a69456d4a69456The three baseline failures are ordinary assertions, not missing imports: main omits the request footer, accepts the moved-head claim instead of refusing it (
0 != 3), and omits the done footer. The branch passes those same cases twice.tests/test_test_requests.pyandtests/test_test_requests_pin.pyalso pass in full.The full suite used
tests/run_all.py --jobs 6 --strict, repository-default compiler and fresh HOME, on 938ab233 after the last commit. Main's suite is the shared baseline run once for this day's moved pull requests ond4a69456. These checks have not yet been run on a forge runner.Main additionally failed two
tests/test_appimage_package_retry.pytimeout/sequence cases under heavy load; its requested isolated rerun passed all 9 tests. It fails only under heavy load on this PC; timing-dependent. This branch passed it. The full suites are not called all green.The current unauthenticated API read resolved main's full
d4a69456SHA and returnedtotal_commits: 1for its parent→head comparison andtotal_commits: 0in reverse. The offline suite separately uses retained real ancestor, identical and divergent responses plus clearly labelled synthetic error cases. Public API reads do not exercise signed-in collaborator lookup, token refresh, room events or Discord rendering.The original GitHub implementation's 270 checks and fork checks belong to that different client. Its later head daad1350 passed the queue tests on Ubuntu and Windows but had documented unrelated whole-suite failures; the earlier d22aec74 run was green. Neither is presented as testing this Forgejo reimplementation. Its historical request-only trial also does not prove the new forge room or relay.
The complete suite was rerun after every source/documentation commit, with the default compiler and fresh HOME. The earlier measurement rows remain labelled by their actual commits. Main's suite was run once for all branches on this commit. No hosted CI or new game run is claimed.
Testing
Tested: on this PC (Arch Linux, i9-13900K; RTX 4090 with NVIDIA 615.78.08 was not used for this game-free change), both complete queue test programs, the whole game-free suite, and the same three central pin cases against main and this PR twice in interleaved order. Authentication, room and HTTP operations were mocked and unexpected live calls failed closed; three separate unauthenticated public GETs checked commit/comparison response shapes. The pointer fixture was also checked alone on both main and this branch after its retained whole-suite failure. Tested but not measured: Public unauthenticated API response shape and offline correctness were checked, not client speed, signed-in room delivery or Discord rendering.
Not tested: the explicitly listed platforms, integration and runtime cases below remain untested. This move used offline tests and public GETs only; the signed-in trial requires separately approved external writes. No cross-platform coverage is claimed.
update, or separating old and current results. The label-generated card producer is also unverified.The original review explicitly left claim/result/update relay behavior open before relying on it. That integration requirement is still open here. Open as draft; offline coverage and public GETs are not a substitute for the signed-in trial.
Needs other testers:
None
Screenshots
Nothing on screen changes in this tool; retained historical pictures, if any, are not main/PR rendering comparisons.
Nothing on screen was tested: offline client output only. No Matrix or Discord screenshot is supplied or claimed.
Risk and rollback
An old client or Discord relay may ignore the new footer/update action, and a wrong pin can misdirect a test. Leave this WIP until the signed-in request/claim/update/done trial is approved and observed. Reverting the commit restores the old client; already posted pin/update events remain in room history, so do not treat a rollback as erasing those records. No game setting changes.
Reviewer notes
Start with
pr_commit,pin_fields,replay,current_results,cmd_claimandcmd_doneinscripts/test_requests.py, then the recorded/synthetic fixture distinction. This is a Forgejo reimplementation, not the old GitHub backend. The original reviewer's live relay requirement remains open; the tester ancestry/diff steps are manual, not an automatic verifier.Issues
Original pull request and review history: https://github.com/NotRexed/Hookshot/issues/2830.
Original related testing discussion: https://github.com/NotRexed/Hookshot/issues/2797.
On this forge: See #90 (the recorder test that failed on Windows while this was checked; this pull request is where it was noticed, not its fix). No old GitHub number is used as a forge closing reference.
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above). The refreshed suite retains the pointer-fixture failure described above.third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run.mainand passing here.tests/that reads changed source updated in this PR, not weakened.Moved from NotRexed/Hookshot#2830.
Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]
Discussion carried over from NotRexed/Hookshot#2830
Every comment that was readable on the original thread when it was copied on 2026-11-12, in order, unchanged except that issue and pull request numbers are written as links to the old repository and names carry no @. Comments by an account that has since been closed there could no longer be read and are missing; where a reply quotes them, the quote remains.
mooki000, 2026-11-12 18:01 UTC (comment, original):
Thanks for the careful read. All four points are right, and I am taking them as you ordered them:
currentview in the JSON output, with both skills andclaim's refusal worded in terms of it, so an agent no longer skips a card because of a result for an older commit.requestrefuses a commit outside the pull request when GitHub can tell.git rev-parsefor the full hash.The nits too: the claim's commit dropped on release and expiry, and note lines that start with
Commit:orHead:.The relay: agreed that it should not merge before the claim, result and
updatemessages have gone through the real room. I will run them on the trial card once the fixes are pushed and ask you to look at how the relay shows them. If the card cannot be claimed without the label, I will ask you for the label on this pull request for that one trial.mooki000, 2026-11-12 18:41 UTC (comment, original):
Thanks; each point is now addressed:
56661df6: JSON exposescurrent_results; both skills and claim refusals distinguish the card's pin from a moved PR head.8652d4b8: request/update verify non-head commits with GitHub. Before building,verifychecks the fetched PR ancestry and prints the pinned diff; an outside commit releases the claim. A real Git fixture covers a fetchable outside commit.6849b187: a moved head sends no claim without explicit--commitconfirmation; unattended loops must skip it. Verification also stops if the head moves after claiming.3a8262d2: the request skill putsgit rev-parse --verify "HEAD^{commit}"beside the token/outage instructions.Nits:
8db228eeclears commit/head snapshots when claims end;daad1350preserves metadata-looking note lines and documents the reserved trailing footer.Rebased cleanly onto
4cf785a0; headdaad1350. No conflicts; all seven patches unchanged in the final rebase.Checks: 270 offline checks pass, including the real old client. The whole Linux suite: 501 passed, 1 failed, 20 skipped; only main's known
test_readme_menu_rows.pyfailure (missing “Skip opening”). Pre-push check: PASS.A full run of the project's checks on this exact head in my fork, Windows included:
tests/test_test_requests.pypasses on Ubuntu and Windows. Red there were only tests this branch does not touch: main's two known ones (test_readme_menu_rows.py, and on Windowstest_wb_shadow_remake.py, fix in NotRexed/Hookshot#2868), plus one Windows failure oftest_recorder_sync.pywhose own output shows a timing-dependent negative case ("vkDestroyBuffer made without waiting ... NOT CAUGHT"); I will look at that one separately.The live claim/update/result trial is still pending. Could you add
testing wantedto NotRexed/Hookshot#2830 for T-6BH3? I will update its old pin to the reviewed head first, then capture each relay step; no game run or unrelated PR is needed.The description now has the current template's Testing section: what was tested, what was not, and why no other hardware testers are requested. It also has one block for the branch's base and suite; nothing else changed.
View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.