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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
