From 45f0faed9cc751be1c9523d92f7200f170f3b318 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Thu, 27 Aug 2026 17:05:51 -0700 Subject: [PATCH] Assert profile-path attribution on call counts, not elapsed time ProfilePaths.SameCalleeUnderTwoCallersGetsSeparateNodes compared the cumulative TIME of the same callee under a light and a heavy caller. That failed a macOS CI run and blocked a merge. It is a genuine flake, not a symptom of anything: heavy does about ten times the work of light, so the ratio is normally around 9x -- but measured over 15 runs it collapsed as low as 1.79x, and on a shared runner it can cross 1.0. Wall clock is not something to assert an ordering on when an exact quantity is available. The exact quantity was already there. Recursion folds onto a child node, and that child carries the call count: 2 under light against 20 under heavy, identical on both engines. That is a stronger statement of the property the test exists for -- each path carries its own attribution -- and it cannot invert. The times are still checked, but only for existence, which is all that is deterministic about them. Checked while here: no other assertion in the file compares timings between two workloads. The rest are >= 0, > 0, or cumulative >= self, which are structural. 60 runs across both engines, no failures. 1078 tests pass. --- pyproject.toml | 2 +- tests/test_profiling.cpp | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 59c9231..7a78ef8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.51.1" +version = "0.51.2" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/tests/test_profiling.cpp b/tests/test_profiling.cpp index 52d831d..ca11c33 100644 --- a/tests/test_profiling.cpp +++ b/tests/test_profiling.cpp @@ -182,9 +182,30 @@ TEST(ProfilePaths, SameCalleeUnderTwoCallersGetsSeparateNodes) { ASSERT_NE(workUnderHeavy, nullptr); // Two distinct nodes for the same callee -- the whole point. EXPECT_NE(workUnderLight, workUnderHeavy); - // ...and the expensive path is attributed the larger share. This is the - // fact the aggregated view cannot express at all. - EXPECT_GT(workUnderHeavy->cumulativeTime, workUnderLight->cumulativeTime); + + // ...and each carries its OWN attribution, which is the fact the + // aggregated view cannot express at all. + // + // Asserted on call counts, not on elapsed time. The comparison here used + // to be EXPECT_GT on cumulativeTime, and it was flaky: the ratio is + // normally about 9x but was measured collapsing to 1.79x, and it failed + // a macOS CI run outright. Wall-clock on a shared runner is not a thing + // to assert an ordering on when an exact quantity is available. + // + // The recursion folds onto a child node (see RecursionFoldsOntoASingleNode), + // and THAT is where the difference in work shows up: 2 recursive calls + // under light against 20 under heavy. Exact, identical on both engines. + const auto* foldedLight = childNamed(r, *workUnderLight, "work"); + const auto* foldedHeavy = childNamed(r, *workUnderHeavy, "work"); + ASSERT_NE(foldedLight, nullptr); + ASSERT_NE(foldedHeavy, nullptr); + EXPECT_EQ(foldedLight->callCount, 2); + EXPECT_EQ(foldedHeavy->callCount, 20); + + // Times are still recorded per path -- just not compared against each + // other, since only their existence is deterministic. + EXPECT_GT(workUnderLight->cumulativeTime, 0.0); + EXPECT_GT(workUnderHeavy->cumulativeTime, 0.0); // The flat view still totals both, unchanged. double siteTotal = 0.0;