borinquenkid commented on PR #16033: URL: https://github.com/apache/grails-core/pull/16033#issuecomment-5148374227
Thanks for the detailed review — all five points below are addressed in `a42b48780f`, but I want to be upfront that the central question you raised isn't resolved, only investigated further. **The 5 inline points:** 1. Blocking (trait `cleanup()`/`cleanupSpec()` breaks downstream specs): reverted. `JsonViewTest` is back to its pre-PR shape. 2. `ConstraintEvalUtils` reset not load-bearing: agreed, moot now that point 1 is reverted — your `ShutdownOperations`/`preserveForNextShutdown` analysis was correct. 3. `cleanupSpec()` no-op: removed along with `@CompileDynamic`. 4. `ExpandSpec`-only decoupling incomplete: extended to `IncludeAssociationsSpec`, `HalEmbeddedSpec` (`Team`/`Player` and `Person`), `IterableRenderSpec`, `MapRenderSpec`, `NullRenderingSpec`, and `JsonApiHandleAssociationsSpec` (`Author`) — each gets its own spec-prefixed entity classes now, so no class is registered into two independently-built mapping contexts. 5. `JsonApiSpec`'s `SuperHero` cache left populated after the last feature: `cleanup()` restored (it only had to go in the first version because the trait declared one). **On your stack-trace ask — still open.** I couldn't produce one either. What I checked: - All ~50 failed CI runs on `8.0.x` in the last 30 days (the window #16030's dashboard covers): zero show a `grails-views-gson` failure. - Ran the four originally-flagged specs together in a single JVM/fork (`-PmaxTestParallel=1 -PforkEveryUnitTest=0`) against the pre-this-PR base commit, 60 iterations (~44 min): 0 failures. Same result you got with your 4 runs. So three independent attempts now (yours, and this one twice) have failed to reproduce it locally, and I couldn't find a corroborating CI job failure either. `testlens-app`'s dashboard is the only source for the failure counts, and its underlying per-test data isn't reachable via the GitHub API or CI artifacts — I couldn't get past the aggregate numbers to see what's actually throwing. Given that, I've updated the PR description to stop asserting the class-cache-leak explanation as settled and instead flag it as the working hypothesis it is. The entity-decoupling change is worth keeping on its own merits (shared `Class` objects across independently-built mapping contexts is a real risk regardless), but I think your whole-spec-fixture-failure theory is still the more likely explanation for the *specific* pattern in #16030, and this PR doesn't confirm or rule it out either way. Open to suggestions on how to get real evidence here — happy to try something more targeted than brute-force repetition if you have an idea for what would actually trigger it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
