Repository navigation
test(agc): register full recorder suite with CTest - #6642
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
fma-rc
left a comment
fma-rc
left a comment
There was a problem hiding this comment.
I built main 5d9dcc4 with this PR (fb67697) and ran the new agc_driver_recorder entry on an RTX 5080, a Radeon 610M (RADV) and lavapipe. It fails on all three, on every run, at (9) a scan under a pending write was kept (core/libs/prx/libSceAgcDriver/tests/Recorder.cpp:1332). The no-argument binary fails the same way on main without the PR (9 of 9 runs, three per device), so the check that blocked the earlier registration still fails, and once the Linux job runs this entry it will go red. Could that check be fixed first, or the registration wait until it passes? I don't think the issue should close on this change alone.
|
Thanks for running the full entry on three devices. I confirmed the reported assertion is in the existing |
|
I found and opened PR #6676 for the pending-writer proof issue behind the |
|
I ran this registration on top of the two PRs on Linux:
So this entry goes green once #5883 and #6155 are merged. "Depends on: #5883, #6155" in place of Disclosure: #6155 is mine, and it is stacked on #5883. AI-assisted: yes |
|
Thanks for testing the exact compositions and separating the two failures. I updated the PR to depend on #5883 and #6155, recorded your 3/3 lavapipe pass with both dependencies, and kept the limitation explicit: this registration alone does not make the main branch's recorder path green. I'll keep it waiting until both dependencies merge and the composed test remains green. |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
The composed run on a hardware GPU, with the PR heads themselves, and how long the entry takes against its 30 s timeout.
Machine: NVIDIA GeForce RTX 3070, driver 595.99.02, Vulkan 1.4.329, plus lavapipe (Mesa 26.0.3, LLVM 21.1.8) on the same machine. Ubuntu 26.04 (kernel 7.0.0-38), i7-8700, GCC 15.2, Release, BUILD_TESTING=ON.
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02), lavapipe (Mesa 26.0.3, LLVM 21.1.8), Ubuntu 26.04
Composition: main 0615cd2b8 + #5883 (1b3f2e098) + #6155 (4aeb8ef49) + this PR (fb67697b7), merged in that order, no conflicts. main did not build at the time (#6784), so I used 0615cd2b8.
ctest -R "^agc_driver_recorder$" -j1, 3 runs per device:
| RTX 3070 | lavapipe | |
|---|---|---|
| this PR alone | fails, (9) a scan under a pending write was kept |
same |
| this PR + #5883 + #6155 | passes 3/3: 1.11 s, 0.71 s, 0.69 s | passes 3/3: 1.26 s, 0.93 s, 0.92 s |
So the full no-argument flow passes on NVIDIA as well, using the actual PR heads (fma-rc's 3/3 in #1532 was rebuilt from a description). The longest run took about 1.3 s, well under the 30 s TIMEOUT this registers.
Not checked: Windows, AMD, the run without #6155 on this machine.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Thanks for running the registered recorder suite on the PR heads themselves and reporting the runtime against its 30-second timeout. This gives useful hardware coverage for the new CTest registration. |
|
The current PR body now has the required What, Tested, and Checklist sections populated and accurately records the dependency and validation status. The earlier conventions failure is from the 00:54 run; later runs at 03:41 and 11:58 passed conventions. No metadata edit is needed here. |
|
Thanks for testing the exact PR head across the three devices. The PR description already records #5883 and #6155 as required dependencies; #6155 is merged, while #5883 is still open. I agree the registration is not ready to merge until #5883 lands and the composed run passes. The independent composed run on #5883 + #6155 + this PR passed 3/3 on RTX 3070 and lavapipe; I am treating that as dependency-stack evidence, not as a pass for this PR alone. |
|
Thanks for adding the RADV result. That is useful composed-stack coverage for the registered |
|
Thanks for the composed-head retest. The result confirms the full recorder registration passes 3/3 on RTX 3070 and lavapipe when #5883 and #6155 are included, and stays well under the 30 s timeout. #5883 is still open and the current main baseline fails the same check, so I’ll keep #5883 listed as a dependency and leave this PR pending until that fix lands; this is not a standalone-main pass. |
fb67697 to
4c1de7e
Compare
|
Rebased onto current |
Register the no-argument recorder executable path so CTest covers the main recorder regression flow in addition to the focused subsets. Preserve the Vulkan-unavailable skip code, cap the test at 30 seconds, and add the Windows libc DLL path used by sibling recorder tests. This carries forward the test registration proposed in boykopovar#3354, which was closed by the intake pause. Its author left a handoff, and the current main branch has since incorporated the fixes that made the full recorder run pass. Co-authored-by: blawesom <blaplane@gmail.com>
4c1de7e to
f9123c7
Compare
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Follow-up to my composed run above, now that #5883 is merged: this PR alone on current main passes.
Setup: Ubuntu 26.04, i7-8700, GCC 15.2, Release. Head f9123c760 merged onto main fe2b52c7e (clean, includes #5883). ANYPS5_NO_SHADER_CACHE=1, ctest -j1, device picked with VK_ICD_FILENAMES.
| RTX 3070 | lavapipe | |
|---|---|---|
agc_driver_recorder |
passes 3/3, 1.45–1.50 s | passes 3/3, 0.99–1.06 s |
same, with #5883's Recorder.cpp change reverted |
fails 3/3, (9) a scan under a pending write was kept |
same |
all 77 entries that run agc_driver_recorder_tests |
77/77 | 77/77 |
So the dependency is resolved and the new entry stays far below its 30 s timeout.
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02), lavapipe (Mesa 26.0.3, LLVM 21.1.8), Ubuntu 26.04
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
What
Register the no-argument
agc_driver_recorderregression flow with CTest, including the existing Vulkan-unavailable skip code, a 30-second timeout, and the WindowslibcDLL path used by focused entries. This carries forward the CTest registration proposed in #3354.This registration depends on #5883. That PR updates the pending-write proof expectation; #6155, which stamps test-side guest writes, has merged. Without #5883, the full recorder path stops at an existing assertion before completing. This PR only registers the test and does not change those expectations or production behavior.
Tested
4c1de7e2b0:git diff --check upstream/main...HEADandpython3 tools/check_conventions.py --base upstream/main --head HEADpassed.3f171d03e, test(agc): account for unknown pending DCC key writes #5883, test(agc): stamp the guest writes the recorder tests make behind the driver's back #6155 and this PR passedctest -R '^agc_driver_recorder$'3/3 on lavapipe. A separate earlier composition with test(agc): account for unknown pending DCC key writes #5883 and test(agc): stamp the guest writes the recorder tests make behind the driver's back #6155 also passed 3/3 on RTX 3070 and lavapipe. These reports are in the PR comments; they predate current main43a2abaf77./dev/dridevice.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch; no silent stubs added