From d704401a561c898676ffeb7fe3845838a7a8fe7a Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 6 Sep 2026 08:36:02 -0400 Subject: [PATCH] [Test] (Pipe): one case per CSO lane, because two of them would race on the lane's log - CsoContentAddressingScenario reads the library's own summary line, and a log is a per-LANE resource: the library opens it fopen(path, "w"), so every process in a lane truncates it. The file had TWO cases in each lane, which under `ctest -j` is a race whose failure mode is an empty read - indistinguishable from "the counters were never emitted", which is precisely the thing the case exists to report on. - The separate plumbing case is folded into the control as its first ASSERT, keeping its own message, so nothing is lost but the flake. Splitting it out bought a clearer failure message and paid for it with a flake in the mechanism that message is about. - This is the same hazard the file's existing comments describe for the arming lane; it is worth saying out loud that the rule is "a log-reading case owns its lane", not "a log-reading case owns its log path". - Verified at -j 4: 44/44 on build-verify and 24/24 on build-push, and the pull/push ctest name lists are still identical (1402 entries each; 0 names removed against the contract tree, 34 added). --- .../CsoContentAddressingScenario.cpp | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp index 9a1d727b..47d6538a 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp @@ -251,30 +251,15 @@ void main() { oColor = vec4(0.0, 1.0, 0.0, 1.0); } GLuint m_vbo = 0; }; - // The plumbing, asserted on its own so that a counter-ratio failure below can never be - // confused with "the lane never turned the stats channel on". - TEST_F(CsoContentAddressingScenario, TheLibrarysSummaryLineCarriesTheCsoCounters) { - if (!Ready()) return; - SkipUnlessTheLaneIsAssertableHere(); - if (IsSkipped()) return; - - Gl().EndFrame(); // close the setup window - RunBlendToggleFrame(); - - const CsoWindow window = LastCsoWindow(ReadWholeFile(LibraryLogPath())); - ASSERT_TRUE(window.found) - << "no 'MGPipe stats:' line carrying cso[csom= csob=] in " << LibraryLogPath() - << ". This IS a push build (the lane checked MGITEST_PIPE_PUSH_BUILD before getting " - "here) and the cso[] bracket is unconditional inside that #if, so the bracket cannot " - "be missing for a build reason: either MOBILEGL_PIPE_STATS / " - "MOBILEGL_PIPE_STATS_PERIOD did not reach the process, or no summary line was " - "emitted at all because nothing reached PipeStats::OnPresent."; - EXPECT_GE(window.binds, 0) << window.line; - EXPECT_GE(window.mints, 0) << window.line; - RecordProperty("cso_line", window.line.c_str()); - } - - // The control itself. + // ONE case per lane, and that is a hard constraint rather than a style choice. + // + // This case READS the library log, and the log is a per-LANE resource: the library opens it + // fopen(path, "w"), so every process in a lane truncates it. A second case in this lane would + // therefore race this one under `ctest -j`, and the shape of the failure is a silent, empty + // read that looks exactly like "the counters were never emitted". Splitting the plumbing + // assertion into its own case would have bought a clearer failure message and paid for it + // with a flake in the thing the message is about. The plumbing is asserted first, with its + // own message, inside this one process instead. TEST_F(CsoContentAddressingScenario, TheBlendToggleMintsBoundedlyWithContentAddressingAndPerBindWithout) { if (!Ready()) return; SkipUnlessTheLaneIsAssertableHere(); @@ -283,8 +268,15 @@ void main() { oColor = vec4(0.0, 1.0, 0.0, 1.0); } Gl().EndFrame(); // close the setup window const Image first = RunBlendToggleFrame(); const CsoWindow window = LastCsoWindow(ReadWholeFile(LibraryLogPath())); - ASSERT_TRUE(window.found) << "no CSO counters in " << LibraryLogPath() - << " - see TheLibrarysSummaryLineCarriesTheCsoCounters"; + // The plumbing first, with its own message, so a counter-ratio failure below can never + // be confused with "the lane never turned the stats channel on". + ASSERT_TRUE(window.found) + << "no 'MGPipe stats:' line carrying cso[csom= csob=] in " << LibraryLogPath() + << ". This IS a push build (the lane checked MGITEST_PIPE_PUSH_BUILD before getting " + "here) and the cso[] bracket is unconditional inside that #if, so it cannot be " + "missing for a build reason: either MOBILEGL_PIPE_STATS / " + "MOBILEGL_PIPE_STATS_PERIOD did not reach the process, or no summary line was " + "emitted at all because nothing reached PipeStats::OnPresent."; RecordProperty("cso_line", window.line.c_str()); // Every draw in the frame changed the pipeline subset, so every draw is a bind. This