You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/01/24 01:27:07 UTC

[PR] Optimize CI workflow to reduce build time [grails-core]

jamesfredley opened a new pull request, #15335:
URL: https://github.com/apache/grails-core/pull/15335

   Replace --rerun-tasks with cleanTest to leverage Develocity cache while still ensuring tests always run.
   
   Changes:
   - Replace --rerun-tasks with cleanTest pattern (enables compilation caching)
   - Add --configuration-cache-problems=warn for faster Gradle startup
   - 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
   
   Ref: https://github.com/apache/grails-core/commit/a78c0225059468f4d66bb13b72f813be2e78a66c


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3795600860

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


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3796883505

   @RestoreSystemProperties will replaced all the system properties changes.


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2733505980


##########
grails-mimetypes/src/main/groovy/org/grails/web/mime/HttpServletRequestExtension.groovy:
##########
@@ -75,7 +76,17 @@ class HttpServletRequestExtension {
     static MimeType[] getMimeTypes(HttpServletRequest request) {
         MimeType[] result = (MimeType[]) request.getAttribute(GrailsApplicationAttributes.REQUEST_FORMATS)
         if (!result) {
-            WebApplicationContext context = WebApplicationContextUtils.getWebApplicationContext(request.servletContext)
+            // First try to get context from GrailsWebRequest (more reliable in tests with parallel execution)

Review Comment:
   Thank you, the comment has been cleaned up so it does not reference tests.  



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2733543889


##########
gradle/test-config.gradle:
##########
@@ -47,10 +61,9 @@ tasks.withType(Test).configureEach {
     useJUnitPlatform()
     jvmArgs += java17moduleReflectionCompatibilityArguments
     develocity {
-        testRetry {
-            maxRetries = configuredTestParallel == 1 ? 1 : 2
-            maxFailures = 20
-            failOnPassedAfterRetry = true
+        testDistribution {
+            enabled = isCiBuild
+            maxLocalExecutors = configuredTestParallel

Review Comment:
   It was previously in place for 'testBisect'



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725862514


##########
grails-test-examples/gsp-spring-boot/script/.sdkmanrc:
##########
@@ -1,3 +1,3 @@
-java=17.0.12-librca
+java=17.0.18-librca

Review Comment:
   The .sdkmanrc file in grails-test-examples/gsp-spring-boot/script/ appears to be for running the standalone Groovy script example with the Groovy CLI, not for building the project.   https://github.com/apache/grails-core/blob/0bd4622d11c2c611dd38c53df3ab659887410ab0/grails-test-examples/gsp-spring-boot/script/README.md



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725802587


##########
.github/workflows/gradle.yml:
##########
@@ -91,7 +100,6 @@ jobs:
         run: >
           ./gradlew build :grails-shell-cli:installDist groovydoc
           --continue
-          --rerun-tasks

Review Comment:
   Based on the commit where `--rerun-tasks` was added, I was working under the assumption that it was added solely to force tests to rerun.  I found that `outputs.cacheIf { !isCiBuild }` works better to force tests to rerun.
   
   Since `--rerun-tasks` is fixing more than just test reruns, is there a specific Gradle task or task that we want to ensure always run?  It is possible to force just a task or small set of tasks to run, which should be faster than `--rerun-tasks`.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725641655


##########
grails-mimetypes/src/main/groovy/org/grails/web/mime/HttpServletRequestExtension.groovy:
##########
@@ -75,7 +76,17 @@ class HttpServletRequestExtension {
     static MimeType[] getMimeTypes(HttpServletRequest request) {
         MimeType[] result = (MimeType[]) request.getAttribute(GrailsApplicationAttributes.REQUEST_FORMATS)
         if (!result) {
-            WebApplicationContext context = WebApplicationContextUtils.getWebApplicationContext(request.servletContext)
+            // First try to get context from GrailsWebRequest (more reliable in tests with parallel execution)

Review Comment:
   I had the same concern until stumbling on the following, there are also a few other reasons in the PR description above, but this was the one that made be lean towards fixing the production code.  
   
   
   **Consistency with `HttpServletResponseExtension`**: The sister class `HttpServletResponseExtension.getMimeTypes()` already uses `GrailsWebRequest.lookup()` as the primary lookup method. This change makes `HttpServletRequestExtension` consistent.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2733530939


##########
gradle/test-config.gradle:
##########
@@ -47,10 +61,9 @@ tasks.withType(Test).configureEach {
     useJUnitPlatform()
     jvmArgs += java17moduleReflectionCompatibilityArguments
     develocity {
-        testRetry {
-            maxRetries = configuredTestParallel == 1 ? 1 : 2
-            maxFailures = 20
-            failOnPassedAfterRetry = true
+        testDistribution {
+            enabled = isCiBuild
+            maxLocalExecutors = configuredTestParallel

Review Comment:
   I am going to carve this off into a research issue since they are not configured, but are supported.  I'll add it to the issue for migrating to Apache's instance.  



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3801083556

   > > Many comments mention `parallel` test execution. I think that might be confusing. We are, to my knowledge, not running tests in parallel in the same process. The parallelism comes from Gradle running multiple forks which are already isolated from each other. https://spockframework.org/spock/docs/2.4/all_in_one.html#parallel-execution
   > 
   > I'll adjust the comments, since fork and parallel have different meanings for these flaky test failures.
   
   I think it might be clearer to talk about test environment pollution.


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2733490268


##########
.github/workflows/gradle.yml:
##########
@@ -91,7 +100,6 @@ jobs:
         run: >
           ./gradlew build :grails-shell-cli:installDist groovydoc
           --continue
-          --rerun-tasks

Review Comment:
   --rerun-tasks has been restored in all jobs except build grails core and a duplicate job that reruns tasks on just Ubuntu with Java 17 has been added.  This should give us the coverage while saving cumulative time. 



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3793605827

   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.


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725769238


##########
grails-test-examples/gsp-spring-boot/script/.sdkmanrc:
##########
@@ -1,3 +1,3 @@
-java=17.0.12-librca
+java=17.0.18-librca

Review Comment:
   Should we even keep .sdkmanrc here?  It was left over from moving this project into the mono repo, yes? 



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728745653


##########
grails-mimetypes/src/main/groovy/org/grails/web/mime/HttpServletRequestExtension.groovy:
##########
@@ -75,7 +76,17 @@ class HttpServletRequestExtension {
     static MimeType[] getMimeTypes(HttpServletRequest request) {
         MimeType[] result = (MimeType[]) request.getAttribute(GrailsApplicationAttributes.REQUEST_FORMATS)
         if (!result) {
-            WebApplicationContext context = WebApplicationContextUtils.getWebApplicationContext(request.servletContext)
+            // First try to get context from GrailsWebRequest (more reliable in tests with parallel execution)

Review Comment:
   OK, but can we remove the mentioning of tests in the comments?



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728975371


##########
.sdkmanrc:
##########
@@ -1,3 +1,3 @@
-java=17.0.17-librca
+java=17.0.18-librca

Review Comment:
   Updated all three for now.  New Issue to find better solutions.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3793614184

   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. 


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725404463


##########
grails-mimetypes/src/test/groovy/org/grails/plugins/web/mime/MimeTypesConfigurationSpec.groovy:
##########
@@ -21,11 +21,17 @@ package org.grails.plugins.web.mime
 import grails.core.DefaultGrailsApplication
 import grails.spring.BeanBuilder
 import grails.web.mime.MimeType
+import org.grails.web.mime.HttpServletResponseExtension
 import org.springframework.context.ApplicationContext
 import spock.lang.Specification
 
 class MimeTypesConfigurationSpec extends Specification {
 
+    void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-mimetypes/src/test/groovy/grails/web/mime/MimeUtilitySpec.groovy:
##########
@@ -30,6 +31,11 @@ import spock.lang.Specification
  */
 class MimeUtilitySpec extends Specification {
 
+    void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-test-suite-uber/src/test/groovy/org/grails/web/codecs/HTMLJSCodecIntegrationSpec.groovy:
##########
@@ -57,6 +57,10 @@ public class HTMLJSCodecIntegrationSpec extends Specification {
         GrailsWebMockUtil.bindMockWebRequest()
         registry = GrailsWebRequest.lookup().getEncodingStateRegistry()
     }
+
+    def cleanup() {
+        org.springframework.web.context.request.RequestContextHolder.resetRequestAttributes()

Review Comment:
   Import `RequestContextHolder`?



##########
grails-validation/src/test/groovy/grails/validation/ValidateableTraitSpec.groovy:
##########
@@ -36,6 +36,16 @@ import java.lang.reflect.Method
  */
 class ValidateableTraitSpec extends Specification {
 
+    /**
+     * Clear the static constraints cache for classes that use shared constraints.
+     * This is necessary because the Validateable trait caches constraints in a static field,
+     * and in parallel test execution, the constraints may be evaluated before configuration
+     * has registered the shared constraints.
+     */
+    void setup() {
+        SharedConstraintsValidateable.clearConstraintsMap()

Review Comment:
   Should we also clean up after?



##########
grails-mimetypes/src/test/groovy/org/grails/web/mime/AcceptHeaderParserSpec.groovy:
##########
@@ -41,6 +42,8 @@ class AcceptHeaderParserSpec extends Specification {
     def config
 
     void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and clean this up afterward?



##########
grails-mimetypes/src/test/groovy/org/grails/web/servlet/mvc/RequestAndResponseMimeTypesApiSpec.groovy:
##########
@@ -53,11 +55,14 @@ class RequestAndResponseMimeTypesApiSpec extends Specification{
     }
 
     void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/DefaultGrailsApplicationAttributesTests.groovy:
##########
@@ -50,6 +50,11 @@ class DefaultGrailsApplicationAttributesTests {
         request.setAttribute(GrailsApplicationAttributes.CONTROLLER, controller)
     }
 
+    @org.junit.jupiter.api.AfterEach

Review Comment:
   Import `AfterEach`?



##########
grails-mimetypes/src/main/groovy/org/grails/web/mime/HttpServletRequestExtension.groovy:
##########
@@ -75,7 +76,17 @@ class HttpServletRequestExtension {
     static MimeType[] getMimeTypes(HttpServletRequest request) {
         MimeType[] result = (MimeType[]) request.getAttribute(GrailsApplicationAttributes.REQUEST_FORMATS)
         if (!result) {
-            WebApplicationContext context = WebApplicationContextUtils.getWebApplicationContext(request.servletContext)
+            // First try to get context from GrailsWebRequest (more reliable in tests with parallel execution)

Review Comment:
   Should we adapt production code for tests? Is it possible to clean up pollution in tests instead?



##########
grails-test-suite-uber/src/test/groovy/org/grails/plugins/web/rest/render/BaseDomainClassRendererSpec.groovy:
##########
@@ -50,12 +50,18 @@ import org.springframework.mock.web.MockHttpServletRequest
 import org.springframework.mock.web.MockHttpServletResponse
 import org.springframework.mock.web.MockServletContext
 import org.springframework.web.context.WebApplicationContext
+import org.grails.web.mime.HttpServletResponseExtension
 import org.springframework.web.context.request.RequestContextHolder
 import org.springframework.web.context.support.GenericWebApplicationContext
 import spock.lang.Specification
 
 abstract class BaseDomainClassRendererSpec extends Specification {
 
+    void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-test-suite-uber/src/test/groovy/org/grails/web/util/StreamCharBufferSpec.groovy:
##########
@@ -66,6 +66,10 @@ class StreamCharBufferSpec extends Specification {
         codecOut=new GrailsPrintWriter(out.getWriterForEncoder(htmlCodecClass.encoder, EncodingStateRegistryLookupHolder.getEncodingStateRegistryLookup().lookup()))
     }
 
+    def cleanup() {
+        org.springframework.web.context.request.RequestContextHolder.resetRequestAttributes()

Review Comment:
   Import `RequestContextHolder`?



##########
grails-validation/src/main/groovy/grails/validation/Validateable.groovy:
##########
@@ -103,6 +103,18 @@ trait Validateable {
         return constraintsMapInternal
     }
 
+    /**
+     * Clears the cached constraints map, forcing re-evaluation on next access.
+     * This is primarily useful in testing scenarios where shared constraints
+     * may need to be re-evaluated after configuration changes.
+     *
+     * @since 7.0
+     */
+    @Generated
+    static void clearConstraintsMap() {

Review Comment:
   This is adding to the public API, preferable (if not breaking) added to 7.1.



##########
grails-rest-transforms/src/test/groovy/org/grails/plugins/web/rest/render/hal/HalJsonRendererSpec.groovy:
##########
@@ -84,7 +86,13 @@ class HalJsonRendererSpec extends Specification {
         ShutdownOperations.runOperations()
     }
 
+    void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-test-suite-web/src/test/groovy/org/grails/web/mime/ContentFormatControllerTests.groovy:
##########
@@ -33,6 +33,11 @@ import spock.lang.Specification
  */
 class ContentFormatControllerTests extends Specification implements ControllerUnitTest<ContentController>, DomainUnitTest<Gizmo> {
 
+    def setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectNoDataSpec.groovy:
##########
@@ -30,6 +31,21 @@ class CommandObjectNoDataSpec extends Specification implements GrailsWebUnitTest
         }
     }}
 
+    /**
+     * Clear the static constraints cache for Artist class.
+     * This is necessary because the Validateable trait caches constraints in a static field,
+     * and in parallel test execution, the constraints may be evaluated before doWithConfig()
+     * has registered the shared constraint 'isProg'.
+     *
+     * Also clear ConstraintEvalUtils.defaultConstraintsMap which caches shared constraints
+     * globally. In parallel test execution, another test's config may have been cached,
+     * causing the 'isProg' shared constraint to not be found.
+     */
+    def setup() {
+        ConstraintEvalUtils.clearDefaultConstraints()

Review Comment:
   These are guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-test-suite-uber/src/test/groovy/org/grails/web/util/WebUtilsTests.groovy:
##########
@@ -49,6 +50,8 @@ class WebUtilsTests {
     @BeforeEach
     void setUp() {
         RequestContextHolder.resetRequestAttributes()
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   This is a guard for previous pollution (that in the best of worlds should not be necessary). Should we be good citizens and also clean this up afterward?



##########
grails-validation/src/test/groovy/grails/validation/ValidateableTraitAdHocSpec.groovy:
##########
@@ -27,6 +27,16 @@ import spock.lang.Specification
 
 class ValidateableTraitAdHocSpec extends Specification {
 
+    /**
+     * Clear the static constraints cache for classes that use shared constraints.
+     * This is necessary because the Validateable trait caches constraints in a static field,
+     * and in parallel test execution, the constraints may be evaluated before configuration
+     * has registered the shared constraints.
+     */
+    void setup() {
+        PersonAdHocSharedConstraintsValidateable.clearConstraintsMap()

Review Comment:
   Should we also clean up after?



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725772142


##########
.github/workflows/gradle.yml:
##########
@@ -91,7 +100,6 @@ jobs:
         run: >
           ./gradlew build :grails-shell-cli:installDist groovydoc
           --continue
-          --rerun-tasks

Review Comment:
   This rerun has cause issues in the functional tests b/c of our transitive dependencies.  I'm fine doing it for the regular build, but we need to introduce a build that doesn't have caching if we do this.  I'd suggest just running it weekly.



##########
.github/workflows/gradle.yml:
##########
@@ -243,10 +251,10 @@ jobs:
         env:
           GITHUB_MAVEN_PASSWORD: ${{ secrets.GITHUB_TOKEN }}
         run: >
-          ./gradlew bootJar cleanTest check
+          ./gradlew bootJar check
           --continue
-          --rerun-tasks

Review Comment:
   Same AST transform issue / rerun issues.



##########
.github/workflows/gradle.yml:
##########
@@ -176,8 +184,8 @@ jobs:
         run: >
           ./gradlew bootJar check
           --continue
-          --rerun-tasks
           --stacktrace
+          --configuration-cache-problems=warn

Review Comment:
   Same as core, I think if the tasks don't rerun then it's going to not rebuild with our AST transforms sometimes, which will cause the tests to execute with the old code.



##########
.github/workflows/gradle.yml:
##########
@@ -210,10 +218,10 @@ jobs:
         env:
           GITHUB_MAVEN_PASSWORD: ${{ secrets.GITHUB_TOKEN }}
         run: >
-          ./gradlew bootJar cleanTest check
+          ./gradlew bootJar check
           --continue
-          --rerun-tasks

Review Comment:
   Same AST transform issue / rerun issues.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3795599889

   Looks pretty good compared to recent runs, although there is significant variability 
   
   <img width="421" height="394" alt="image" src="https://github.com/user-attachments/assets/619839c4-d6bb-433a-8e36-5b0007406387" />
   
   vs
   
   <img width="423" height="623" alt="image" src="https://github.com/user-attachments/assets/66708305-ac62-4c2e-9d85-ee5ae4c97ebb" /> 
   <img width="407" height="625" alt="image" src="https://github.com/user-attachments/assets/fba0c267-e801-471d-92e3-f605b078e687" /> 
   <img width="411" height="622" alt="image" src="https://github.com/user-attachments/assets/5f80b702-473c-42d9-ab88-73c28824c70e" />
   
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728641278


##########
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectNoDataSpec.groovy:
##########
@@ -20,16 +20,64 @@
 package org.grails.web.commandobjects
 
 import grails.testing.web.GrailsWebUnitTest
+import org.grails.validation.ConstraintEvalUtils
+import spock.lang.Isolated
 import spock.lang.Specification
 
+/**
+ * Tests for command object validation without DataTest trait.
+ * This spec is marked @Isolated because it modifies global shared constraints
+ * via doWithConfig() which affects ConstraintEvalUtils.defaultConstraintsMap - a static
+ * cache shared across all tests in the same JVM fork.
+ */
+@Isolated

Review Comment:
   I see, will refactor another way.  



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3814614685

   This fixed https://github.com/apache/grails-core/issues/14319


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3793383435

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


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725864283


##########
grails-test-suite-uber/src/test/groovy/org/grails/web/codecs/HTMLJSCodecIntegrationSpec.groovy:
##########
@@ -57,6 +57,10 @@ public class HTMLJSCodecIntegrationSpec extends Specification {
         GrailsWebMockUtil.bindMockWebRequest()
         registry = GrailsWebRequest.lookup().getEncodingStateRegistry()
     }
+
+    def cleanup() {
+        org.springframework.web.context.request.RequestContextHolder.resetRequestAttributes()

Review Comment:
   fixed



##########
grails-test-suite-uber/src/test/groovy/org/grails/web/util/StreamCharBufferSpec.groovy:
##########
@@ -66,6 +66,10 @@ class StreamCharBufferSpec extends Specification {
         codecOut=new GrailsPrintWriter(out.getWriterForEncoder(htmlCodecClass.encoder, EncodingStateRegistryLookupHolder.getEncodingStateRegistryLookup().lookup()))
     }
 
+    def cleanup() {
+        org.springframework.web.context.request.RequestContextHolder.resetRequestAttributes()

Review Comment:
   fixed



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728633543


##########
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectsSpec.groovy:
##########
@@ -23,11 +23,23 @@ import grails.artefact.Artefact
 import grails.testing.gorm.DataTest
 import grails.testing.web.controllers.ControllerUnitTest
 import grails.validation.Validateable
+import org.grails.validation.ConstraintEvalUtils
+import spock.lang.Isolated
 import spock.lang.Issue
 import spock.lang.Specification
 
+/**
+ * Tests for command object binding and validation.
+ * This spec is marked @Isolated because it modifies global shared constraints
+ * via doWithConfig() which affects ConstraintEvalUtils.defaultConstraintsMap - a static
+ * cache shared across all tests in the same JVM fork.
+ */
+@Isolated

Review Comment:
   My understanding of `@Isolated` is that it is only useful when running Spock with parallel execution (https://spockframework.org/spock/docs/2.4/all_in_one.html#parallel-execution). We are not running parallel execution, we are running multiple forks (separate JVM processes, each running one Spec at a time), so Spock will run tests sequentially, in each fork.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725876631


##########
grails-validation/src/main/groovy/grails/validation/Validateable.groovy:
##########
@@ -103,6 +103,18 @@ trait Validateable {
         return constraintsMapInternal
     }
 
+    /**
+     * Clears the cached constraints map, forcing re-evaluation on next access.
+     * This is primarily useful in testing scenarios where shared constraints
+     * may need to be re-evaluated after configuration changes.
+     *
+     * @since 7.0
+     */
+    @Generated
+    static void clearConstraintsMap() {

Review Comment:
   Agreed, will create something which fixes tests another way for now and potentially create another PR with this method against 7.1.x.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3800923860

   > Many comments mention `parallel` test execution. I think that might be confusing. We are, to my knowledge, not running tests in parallel in the same process. The parallelism comes from Gradle running multiple forks which are already isolated from each other. https://spockframework.org/spock/docs/2.4/all_in_one.html#parallel-execution
   
   I'll adjust the comments, since fork and parallel have different meanings for these flaky test failures. 


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728709519


##########
gradle/test-config.gradle:
##########
@@ -47,10 +61,9 @@ tasks.withType(Test).configureEach {
     useJUnitPlatform()
     jvmArgs += java17moduleReflectionCompatibilityArguments
     develocity {
-        testRetry {
-            maxRetries = configuredTestParallel == 1 ? 1 : 2
-            maxFailures = 20
-            failOnPassedAfterRetry = true
+        testDistribution {
+            enabled = isCiBuild
+            maxLocalExecutors = configuredTestParallel

Review Comment:
   Do we have remote agents available/configured?



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3805893712

   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.


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728631972


##########
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectNoDataSpec.groovy:
##########
@@ -20,16 +20,64 @@
 package org.grails.web.commandobjects
 
 import grails.testing.web.GrailsWebUnitTest
+import org.grails.validation.ConstraintEvalUtils
+import spock.lang.Isolated
 import spock.lang.Specification
 
+/**
+ * Tests for command object validation without DataTest trait.
+ * This spec is marked @Isolated because it modifies global shared constraints
+ * via doWithConfig() which affects ConstraintEvalUtils.defaultConstraintsMap - a static
+ * cache shared across all tests in the same JVM fork.
+ */
+@Isolated

Review Comment:
   My understanding of `@Isolated` is that it is only useful when running Spock with parallel execution (https://spockframework.org/spock/docs/2.4/all_in_one.html#parallel-execution). We are not running parallel execution, we are running multiple forks (separate JVM processes, each running one Spec at a time), so Spock will run tests sequentially, in each fork.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley merged PR #15335:
URL: https://github.com/apache/grails-core/pull/15335


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725864113


##########
grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectNoDataSpec.groovy:
##########
@@ -30,6 +31,21 @@ class CommandObjectNoDataSpec extends Specification implements GrailsWebUnitTest
         }
     }}
 
+    /**
+     * Clear the static constraints cache for Artist class.
+     * This is necessary because the Validateable trait caches constraints in a static field,
+     * and in parallel test execution, the constraints may be evaluated before doWithConfig()
+     * has registered the shared constraint 'isProg'.
+     *
+     * Also clear ConstraintEvalUtils.defaultConstraintsMap which caches shared constraints
+     * globally. In parallel test execution, another test's config may have been cached,
+     * causing the 'isProg' shared constraint to not be found.
+     */
+    def setup() {
+        ConstraintEvalUtils.clearDefaultConstraints()

Review Comment:
   Added



##########
grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/DefaultGrailsApplicationAttributesTests.groovy:
##########
@@ -50,6 +50,11 @@ class DefaultGrailsApplicationAttributesTests {
         request.setAttribute(GrailsApplicationAttributes.CONTROLLER, controller)
     }
 
+    @org.junit.jupiter.api.AfterEach

Review Comment:
   fixed



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725863978


##########
grails-validation/src/test/groovy/grails/validation/ValidateableTraitSpec.groovy:
##########
@@ -36,6 +36,16 @@ import java.lang.reflect.Method
  */
 class ValidateableTraitSpec extends Specification {
 
+    /**
+     * Clear the static constraints cache for classes that use shared constraints.
+     * This is necessary because the Validateable trait caches constraints in a static field,
+     * and in parallel test execution, the constraints may be evaluated before configuration
+     * has registered the shared constraints.
+     */
+    void setup() {
+        SharedConstraintsValidateable.clearConstraintsMap()

Review Comment:
   Added



##########
grails-validation/src/test/groovy/grails/validation/ValidateableTraitAdHocSpec.groovy:
##########
@@ -27,6 +27,16 @@ import spock.lang.Specification
 
 class ValidateableTraitAdHocSpec extends Specification {
 
+    /**
+     * Clear the static constraints cache for classes that use shared constraints.
+     * This is necessary because the Validateable trait caches constraints in a static field,
+     * and in parallel test execution, the constraints may be evaluated before configuration
+     * has registered the shared constraints.
+     */
+    void setup() {
+        PersonAdHocSharedConstraintsValidateable.clearConstraintsMap()

Review Comment:
   Added



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725863706


##########
grails-validation/src/main/groovy/grails/validation/Validateable.groovy:
##########
@@ -103,6 +103,18 @@ trait Validateable {
         return constraintsMapInternal
     }
 
+    /**
+     * Clears the cached constraints map, forcing re-evaluation on next access.
+     * This is primarily useful in testing scenarios where shared constraints
+     * may need to be re-evaluated after configuration changes.
+     *
+     * @since 7.0
+     */
+    @Generated
+    static void clearConstraintsMap() {

Review Comment:
   Updated.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2730343277


##########
.sdkmanrc:
##########
@@ -1,3 +1,3 @@
-java=17.0.17-librca
+java=17.0.18-librca

Review Comment:
   Dockerfiles can take args, creating a shell script to spin up the verification container is completely reasonable.  Also, the gradle properties github action can load property files like sdkman, so both the release workflow & dockerfile versions should be easily loaded from .sdkmanrc.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725816632


##########
.github/workflows/gradle.yml:
##########
@@ -91,7 +100,6 @@ jobs:
         run: >
           ./gradlew build :grails-shell-cli:installDist groovydoc
           --continue
-          --rerun-tasks

Review Comment:
   The problem is it depends where the change is; that's why I think it's better to take the caching approach and just create a separate workflow that we can check periodically.
   
   If we clean up our dependencies, this should not be an issue longer term.  I think it's transitive while gradle doesn't fully understand our outputs.
   



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725926469


##########
.github/workflows/gradle.yml:
##########
@@ -91,7 +100,6 @@ jobs:
         run: >
           ./gradlew build :grails-shell-cli:installDist groovydoc
           --continue
-          --rerun-tasks

Review Comment:
   Another potential solution, matching how the PR forces tests to rerun.  https://github.com/apache/grails-core/pull/15335/commits/a9bb5730658f3e489f41a227dd4c9a2ed87f0248



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3796451446

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


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725802587


##########
.github/workflows/gradle.yml:
##########
@@ -91,7 +100,6 @@ jobs:
         run: >
           ./gradlew build :grails-shell-cli:installDist groovydoc
           --continue
-          --rerun-tasks

Review Comment:
   Based on the commit where `--rerun-tasks` was added, I was working under the assumption that it was added solely to force tests to rerun.  I found is that `outputs.cacheIf { !isCiBuild }` works better to force tests to rerun.
   
   Since `--rerun-tasks` is fixing more than just test reruns, is there a specific Gradle task or task that we want to ensure always run?  It is possible to force just a task or small set of tasks to run, which should be faster than `--rerun-tasks`.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725912454


##########
grails-validation/src/main/groovy/grails/validation/Validateable.groovy:
##########
@@ -103,6 +103,18 @@ trait Validateable {
         return constraintsMapInternal
     }
 
+    /**
+     * Clears the cached constraints map, forcing re-evaluation on next access.
+     * This is primarily useful in testing scenarios where shared constraints
+     * may need to be re-evaluated after configuration changes.
+     *
+     * @since 7.0
+     */
+    @Generated
+    static void clearConstraintsMap() {

Review Comment:
   Updated and moved public API change to https://github.com/apache/grails-core/pull/15346



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725770979


##########
grails-validation/src/main/groovy/grails/validation/Validateable.groovy:
##########
@@ -103,6 +103,18 @@ trait Validateable {
         return constraintsMapInternal
     }
 
+    /**
+     * Clears the cached constraints map, forcing re-evaluation on next access.
+     * This is primarily useful in testing scenarios where shared constraints
+     * may need to be re-evaluated after configuration changes.
+     *
+     * @since 7.0
+     */
+    @Generated
+    static void clearConstraintsMap() {

Review Comment:
   We should probably be more explicit about this name - it's only clearing a cache.  I'd suggest the name `clearConstraintsMapCache()` 



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2725794649


##########
grails-mimetypes/src/test/groovy/grails/web/mime/MimeUtilitySpec.groovy:
##########
@@ -30,6 +31,11 @@ import spock.lang.Specification
  */
 class MimeUtilitySpec extends Specification {
 
+    void setup() {
+        // Clear the static mimeTypes cache to ensure proper test isolation in parallel test runs
+        HttpServletResponseExtension.@mimeTypes = null

Review Comment:
   Good point, will update all 8 locations.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3800908166

   Many comments mention `parallel` test execution. I think that might be confusing. We are, to my knowledge, not running tests in parallel in the same process. The parallelism comes from Gradle running multiple forks which are already isolated from each other.
   https://spockframework.org/spock/docs/2.4/all_in_one.html#parallel-execution


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728835310


##########
.sdkmanrc:
##########
@@ -1,3 +1,3 @@
-java=17.0.17-librca
+java=17.0.18-librca

Review Comment:
   Remember to also keep this in sync with `.github/workflows/release.yml` and `etc/bin/Dockerfile`. Maybe add a comment here as a reminder?



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Flaky Build Test Cleanup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#discussion_r2728878356


##########
.sdkmanrc:
##########
@@ -1,3 +1,3 @@
-java=17.0.17-librca
+java=17.0.18-librca

Review Comment:
   https://github.com/apache/grails-core/issues/15352



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3793674082

   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.  


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Optimize CI workflow to reduce build time [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15335:
URL: https://github.com/apache/grails-core/pull/15335#issuecomment-3795056920

   <img width="424" height="401" alt="image" src="https://github.com/user-attachments/assets/18c8b343-41c1-4b81-83b7-6b09e8b1ec07" />
   
   encouraging!


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org