Flaky Build Test Cleanup

[jamesfredley]

Summary

This PR optimizes CI workflow performance, fixes multiple flaky tests caused by static state pollution during parallel test execution, and adds developer experience improvements. After fixing the root causes of test flakiness, Develocity test retries have been removed.

CI Workflow Optimizations

Build Performance

• Use --rerun-tasks flag for reliable CI builds
• Disable test caching in CI via outputs.cacheIf { !isCiBuild }
• Disable Groovy compilation cache in CI (GroovyCompile tasks) to ensure AST transformations are always applied
• Increase maxParallelForks from 3 to 4 (matches GitHub runner cores)
• Increase forkEvery from 10-25 to 50 (reduces JVM restart overhead)
• Enable Develocity test distribution for CI builds
• Remove hardcoded maxParallelForks = 1 restriction from grails-gsp module
• Enable cancel-in-progress for PR workflow runs - Cancels outdated workflow runs when new commits are pushed to a PR across all CI workflows (gradle.yml, codestyle.yml, codeql.yml, groovy-joint-workflow.yml, rat.yml)
• Add new buildRerunTasks job for building Grails-Core with rerun tasks enabled

CI Matrix Reduction

• Build job: Changed from full OS/Java cross-product to specific combinations:
  • Ubuntu: Java 17, 21, 25
  • macOS: Java 21 only
  • Windows: Java 25 only
• Other jobs (buildGradle, functional, mongodbFunctional, hibernate5Functional): Changed from [17, 21] to [17, 25]
• Grails Forge build: Changed to [17, 21]

Test Retry Removal

After fixing the flaky tests, Develocity test retries have been removed from:

• gradle/test-config.gradle - Removed testRetry block
• build-logic/docs-core/build.gradle - Removed develocity testRetry block
• grails-data-hibernate5/dbmigration/build.gradle - Removed entire testRetry task configuration
• grails-gradle/gradle/test-config.gradle - Removed develocity testRetry block
• grails-forge/buildSrc/build.gradle - Removed unused test-retry plugin dependency
• grails-forge/gradle/test-config.gradle - Removed commented-out retry code and plugin apply

Flaky Test Fixes

Root Cause Analysis

When tests run in parallel (maxParallelForks > 1), static state can leak between tests causing intermittent failures. The main culprits identified:

1. HttpServletResponseExtension.@mimeTypes - Static cache initialized by one test used by another
2. HttpServletResponseExtension.getMimeTypes() - Was calling webRequest.applicationContext without null check, causing NPE when GrailsWebRequest.lookup() returns null during parallel test runs
3. HttpServletRequestExtension.getMimeTypes() - Was using servlet context attributes which get polluted by parallel tests due to GrailsUnitTest static fields
4. Holders state - grailsApplication, pluginManager cleared mid-test by parallel cleanup
5. Validateable.constraintsMapInternal - Constraint cache evaluated before doWithConfig() registers shared constraints
6. ConstraintEvalUtils.defaultConstraintsMap - Global shared constraints cache keyed by config identity hash
7. SystemPropertyTenantResolver - System property gorm.tenantId pollution between multi-tenancy tests
8. RequestContextHolder - ThreadLocal request attributes not properly reset
9. GrailsApplication.artefactInfo - URL mapping artefacts polluting parallel tests
10. LinkGenerator.urlMappingsHolder - Stale reference to old URL mappings holder after bean recreation

Key Fix Pattern: Holders.clear() in Cleanup

Problem: Calling Holders.clear() in cleanup() or @AfterEach clears ALL Holders state (including grailsApplication) which can affect parallel tests that depend on Holders.grailsApplication for mime type resolution.

Solution:

• Use targeted cleanup (only reset what you set)
• Or use cleanupSpec()/@AfterAll which runs after all tests in the class complete
• The GrailsUnitTest trait already handles Holders.clear() in cleanupClass()

Key Fix Pattern: Ensure grailsApplication Initialization Before Clearing Caches

Problem: Clearing static caches in setup() before grailsApplication is initialized causes doWithConfig() to not be called, meaning custom MIME types and shared constraints are not registered.

Solution: Access the config property first in setup() to trigger lazy initialization of grailsApplication, which calls doWithConfig(). Then clear the caches so they will be repopulated from the fresh config.

Fix Categories

1. MIME Type Lookup Isolation (Production Code Fixes)

HttpServletResponseExtension.getMimeTypes() was calling webRequest.applicationContext without checking if webRequest is null. When GrailsWebRequest.lookup() returns null during parallel test execution, this caused a NullPointerException.

Fix: Added null-safe operator (?.) when accessing webRequest.applicationContext.

HttpServletRequestExtension.getMimeTypes() was using WebApplicationContextUtils.getWebApplicationContext(request.servletContext) to look up MIME types. This fails in parallel tests because GrailsUnitTest uses static _servletContext which gets polluted across tests.

Fix: Now prioritizes GrailsWebRequest.lookup() (thread-local) before falling back to servlet context lookup.

2. Mime Type Cache Isolation (~20 files)

Added setup() and cleanup() methods to clear HttpServletResponseExtension.@mimeTypes before/after each test, ensuring grailsApplication is initialized first:

• RespondMethodSpec, WithFormatContentTypeSpec, ContentFormatControllerTests
• ContentNegotiationSpec, JSONBindingToNullSpec, MimeUtilitySpec
• MimeTypesConfigurationSpec, AcceptHeaderParserSpec, RequestAndResponseMimeTypesApiSpec
• HalJsonRendererSpec, JsonRendererSpec, BaseDomainClassRendererSpec
• RestfulControllerSpec, WithFormatSpec, DefaultXmlRendererSpec
• VndErrorRenderingSpec, HtmlRendererSpec, DefaultRendererRegistrySpec
• And others across modules

3. Multi-Tenancy Test Isolation (~18 files)

Used Spock @RestoreSystemProperties annotation for cleaner system property isolation, combined with clearing SystemPropertyTenantResolver:

• DatabasePerTenantSpec, SchemaPerTenantSpec, PartitionMultiTenancySpec
• MultiTenancySpec, SingleTenantSpec, SchemaMultiTenantSpec
• MongoStaticApiMultiTenancySpec, SchemaBasedMultiTenancySpec, SingleTenancySpec (MongoDB)
• MongoConnectionSourcesSpec, MultiTenancySpec (MongoDB)
• CurrentTenantTransformSpec, TenantServiceSpec, MultiTenantServiceTransformSpec
• And Hibernate/MongoDB connection specs

4. Command Object Constraint Isolation (~15 files)

Tests that use shared constraints via doWithConfig() need proper cache clearing. Uses reflection to clear the private constraintsMapInternal field in the Validateable trait:

• CommandObjectsSpec.groovy, CommandObjectNoDataSpec.groovy
• ValidateableTraitSpec.groovy, ValidateableTraitAdHocSpec.groovy
• CommandObjectConstraintGettersSpec, SerializableValidateableSpec
• ValidateableMockSpec, DefaultASTValidateableHelperSpec
• JsonApiSpec, CommandObjectNullabilitySpec
• ControllerUnitTestMixinTests, JsonBindingWithExceptionHandlerSpec
• And others

Note: A public API method clearConstraintsMapCache() will be added in Grails 7.1 (see PR #15346). This PR uses reflection to access the Groovy 4 trait static field helper interface to clear the cache without modifying the public API, which would be a breaking change in a patch release.

5. Holders.clear() Fixes (4 files)

Replaced aggressive Holders.clear() with targeted cleanup:

• WithFormatContentTypeSpec - Removed Holders.clear() from cleanup()
• BeanBuilderTests - Changed to Holders.setPluginManager null (only reset what setUp sets)
• DomainClassTraitSpec - Moved Holders.clear() from cleanup() to cleanupSpec()

6. RequestContextHolder Cleanup (~10 files)

Added RequestContextHolder.resetRequestAttributes() in setup/cleanup:

• LinkGeneratorWithUrlMappingsSpec, EncodePathFromURISpec
• HTMLJSCodecIntegrationSpec, DefaultGrailsApplicationAttributesTests
• FlashScopeWithErrorsTests, StreamCharBufferSpec, WebUtilsTests
• BindingOutsideRequestSpec
• UrlMappingsWithHttpMethodSpec, RestfulUrlMappingSpec

7. URL Mapping Artefact Isolation (~4 files)

Clear URL mapping artefacts and refresh linkGenerator urlMappingsHolder reference:

• FormTagLibTests - Clear artefactInfo in setup/cleanup, refresh linkGenerator
• AbstractGrailsTagTests - Clear URL mappings in tearDown() for all subclasses
• RestfulReverseUrlRenderingTests - Clear URL mappings and re-register in setup
• ControllerUnitTestMixin.mockArtefact() - Refresh linkGenerator.urlMappingsHolder after bean recreation

8. System Property Isolation (~20 files)

Fixed tests to use @RestoreSystemProperties for proper system property isolation:

• UrlMappingMatcherSpec, YamlPropertySourceLoaderSpec
• NavigableMapSpringProfilesSpec, GrailsPlaceHolderConfigurerCorePluginRuntimeSpec
• DevelopmentModeWatchSpec, GrailsArtefactTransformerSpec
• CreateAppCommandSpec, BookSpec
• Neo4j and MongoDB auto-configuration specs
• And others

Dependency Updates

• Groovy 3.0.25 in grails-data-graphql, grails-data-neo4j, grails-forge (Java 25 compatibility)
• Gradle wrapper 8.14.4 for generated apps (gradleWrapperProperties.rocker.raw)
• Gradle tooling API 8.14.4 (gradle.properties)
• .sdkmanrc updates for Java 17.0.18-librca

Related PRs

• Add clearConstraintsMapCache() public API to Validateable trait #15346 - Adds clearConstraintsMapCache() public API to Validateable trait (targeting 7.1.x)

Files Changed

• ~100 files modified across flaky test fixes, CI optimizations, test retry removal, and production code improvements

[jamesfredley]

First run: https://github.com/apache/grails-core/actions/runs/21306755309?pr=15335

[jdaugherty]

In my early testing, using the cache was not always consistent because of how some of our dependencies are included. Part of the reason we force a rerun of tests was to ensure everything was functional. I am ok disabling the cache for every commit, but I think we still need to run all of the tests periodically.

I would strongly urge we fix the reason the develocity plugin retries the tests first - sometimes it reruns all of the tests 3+ times. Most of the tests that fail are in grails-gsp. If you checkout the last build, look at flaky tests to find the problematic ones. I suspect if we fixed these, the github build would go down by over half of its run time.

[jdaugherty]

Oh and this scenario too: changing something in core might only be tested by the end to end test in forge, but forge doesn't depend on grails so gradle would just not run those tests because the cache would see nothing changed. We had lots of issues earlier on in 7.0 dev that were only caught by the functional tests and gradle would not run them always because the cache would not trigger a change. This was a reoccurring theme that I ended up wasting a lot of time chasing in 7.0 dev.

[jamesfredley]

Apparently cleanTest will only force the rerun of tests when Gradle cache is disabled. I'm trying another approach.

Still experiments to see if anything reduces this from an hour, before proposing changes.

[Copilot]

Pull request overview

This PR optimizes the CI workflow to reduce build times and fixes multiple flaky tests caused by static state pollution during parallel test execution.

Changes:

• Optimized CI workflow by replacing --rerun-tasks with selective test cache disabling, adding configuration cache warnings, and increasing parallel execution settings
• Reduced CI matrix combinations to test specific OS/Java pairings instead of full cross-products
• Fixed flaky tests by adding proper cleanup of ThreadLocal state, static constraints caches, mime type caches, and tenant ID system properties in test setup methods
• Added clearConstraintsMap() method to the Validateable trait to support constraint cache clearing in tests
• Updated Groovy version to 3.0.25 and Gradle wrapper to 8.14.4 for Java 25 compatibility

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
.github/workflows/gradle.yml	Optimized CI workflow by changing Java matrix combinations, removing --rerun-tasks and cleanTest, adding --configuration-cache-problems=warn
build.gradle	Increased configuredTestParallel from 3 to 4 for CI builds to match GitHub runner cores
gradle/test-config.gradle	Added test cache disabling for CI builds, enabled test distribution, increased forkEvery from 20 to 50
grails-test-suite-web/build.gradle	Increased forkEvery from 10 to 50 for CI builds
grails-test-suite-uber/build.gradle	Increased forkEvery from 25 to 50 for CI builds
grails-validation/src/main/groovy/grails/validation/Validateable.groovy	Added clearConstraintsMap() static method to support test isolation
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/LinkGeneratorWithUrlMappingsSpec.groovy	Added setup/cleanup methods to reset RequestContextHolder and link map state
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectsSpec.groovy	Added setup method to clear constraints cache before each test
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectNoDataSpec.groovy	Added setup method to clear constraints cache before each test
grails-test-suite-web/src/test/groovy/org/grails/web/mime/WithFormatContentTypeSpec.groovy	Added setup method to clear mime types cache and set Holders.grailsApplication
grails-test-suite-web/src/test/groovy/org/grails/web/mime/ContentFormatControllerTests.groovy	Added setup method to clear mime types cache
grails-test-suite-web/src/test/groovy/org/grails/web/controllers/ContentNegotiationSpec.groovy	Added setup method to clear mime types cache
grails-test-suite-web/src/test/groovy/grails/rest/web/RespondMethodSpec.groovy	Added setup method to clear mime types cache
grails-test-suite-web/src/test/groovy/org/grails/web/binding/JSONBindingToNullSpec.groovy	Added setup method to clear mime types cache
grails-datamapping-core-test/src/test/groovy/grails/gorm/services/multitenancy/database/DatabasePerTenantSpec.groovy	Added setup method to clear tenant ID system property
grails-forge/gradle.properties	Updated Groovy version from 3.0.24 to 3.0.25
grails-data-neo4j/gradle.properties	Updated Groovy version from 3.0.20 to 3.0.25
grails-data-graphql/gradle.properties	Updated Groovy version from 3.0.11 to 3.0.25
grails-forge/grails-forge-core/src/main/java/org/grails/forge/feature/build/gradle/templates/gradleWrapperProperties.rocker.raw	Updated Gradle wrapper from 8.14.3 to 8.14.4

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jamesfredley]

Looks pretty good compared to recent runs, although there is significant variability

vs

[jamesfredley]

This ended up being more of a test cleanup than a speed improvement PR.

[matrei]

Could we leverage https://spockframework.org/spock/docs/2.4/all_in_one.html#_restoresystemproperties instead of clearing system properties explicitly?

[matrei]

This is good stuff, thank you @jamesfredley

[jamesfredley]

@RestoreSystemProperties will replace all the system properties changes.

[jdaugherty]

My main concern is with the removal of rerun-tasks, we won't always reapply our AST transformers. We experienced this in the 7.x development and a lot of time was lost trying to figure out when this broke. I think this can easily be solved with a build without caching that runs weekly or something.

Thank you for doing all of the test cleanup- it's been sorely needed.

[jamesfredley]

all the version updates were moved out to a separate PR and merged, since they were missed when we updated Gradle, Groovy and Java versions.

Updated

[jdaugherty]

This fixed #14319