You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by jd...@apache.org on 2026/08/25 14:53:15 UTC
(grails-core) branch revert/16192-async-changes created (now e4cd44a3f7)
This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a change to branch revert/16192-async-changes
in repository https://gitbox.apache.org/repos/asf/grails-core.git
at e4cd44a3f7 Run the isolated tests, and see their results
This branch includes the following new commits:
new e8f568d8f4 Revert the async changes from #16192
new 49cbe1d312 Cache the mongod apart from the dependency jars
new cbc0ebb3c7 Run the joint workflow against one MongoDB
new e4cd44a3f7 Run the isolated tests, and see their results
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(grails-core) 03/04: Run the joint workflow against one MongoDB
Posted by jd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a commit to branch revert/16192-async-changes
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit cbc0ebb3c75f145fb33303346a6a61ca784860c3
Author: James Daugherty <jd...@jdresources.net>
AuthorDate: Tue Aug 25 10:21:23 2026 -0400
Run the joint workflow against one MongoDB
The build_grails job served the test-example applications from a mongo:8 service
container on 27017, but named no version to the build, so the specifications that
bring their own server took the default in AbstractMongoGrailsExtension instead:
testcontainers started mongo:7.0.19 and the embedded server started V7_0. One job
was testing three MongoDB releases at once, and which one a failure belonged to
was not apparent from the job.
It now asks for 8.0 in all three places. This workflow validates Grails against a
Groovy snapshot rather than against a range of MongoDB releases -- that coverage
is the mongodb-version matrix in gradle.yml, which runs 7.0 and 8.0 -- so the
latest supported line is the one to run here, and the service container already
said as much. mongo:8 becomes mongo:8.0 so the tag names the same line the
property does rather than a floating major.
---
.github/workflows/groovy-joint-workflow.yml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/.github/workflows/groovy-joint-workflow.yml b/.github/workflows/groovy-joint-workflow.yml
index 5aba28cd7a..208645d76c 100644
--- a/.github/workflows/groovy-joint-workflow.yml
+++ b/.github/workflows/groovy-joint-workflow.yml
@@ -117,7 +117,11 @@ jobs:
runs-on: ubuntu-latest
services:
mongodb:
- image: mongo:8
+ # The test-example applications default to localhost:27017, and this serves them. The
+ # version must stay the same as -PmongodbContainerVersion below: that property is what the
+ # testcontainers-backed specifications and the embedded server read, and when the three
+ # disagree one job tests three different MongoDB releases at once.
+ image: mongo:8.0
ports:
- 27017:27017
options: >-
@@ -161,11 +165,9 @@ jobs:
uses: actions/cache@v4
with:
# Separate from the dependency jars, for the reasons given on the same step in
- # gradle.yml. This job names no version, so it gets the one
- # AbstractMongoGrailsExtension.DEFAULT_MONGO_VERSION names; a change there costs one
- # download, not a wrong binary, since this key has no other reader.
+ # gradle.yml. The version is the one -PmongodbContainerVersion asks for below.
path: ~/.embedmongo
- key: embedmongo-${{ runner.os }}-7.0
+ key: embedmongo-${{ runner.os }}-8.0
- name: "🐘 Setup Gradle"
uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
with:
@@ -197,6 +199,7 @@ jobs:
-PmaxTestParallel=3
-PtestShardCount=3
-PtestShardIndex=${{ matrix.shard_index }}
+ -PmongodbContainerVersion=8.0
env:
GRAILS_INCLUDE_MAVEN_LOCAL: true
- name: "🗄️ Save dependency jar cache"
(grails-core) 02/04: Cache the mongod apart from the dependency jars
Posted by jd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a commit to branch revert/16192-async-changes
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 49cbe1d312ec5c47c32593b99df64196ff70dfc6
Author: James Daugherty <jd...@jdresources.net>
AuthorDate: Tue Aug 25 10:18:18 2026 -0400
Cache the mongod apart from the dependency jars
#16192 added ~/.embedmongo to all seventeen dependency-jar cache steps across the
two workflows. Only two jobs ever run a mongod, and neither of them can write
that cache:
- gradle.yml `build` is the only writer of the gradle-deps key, and it runs
-PonlyCoreTests, which is in the onlyIf of mongodb-test-config.gradle: no
MongoDB test task runs there, so it saved a directory that was never created.
- mongodbFunctional and the joint workflow do download a mongod, but they reach
the same key, and it exists by then, so their post-job save is skipped and
the binary is discarded every run.
Meanwhile the other thirteen jobs restored up to 250MB of mongod they have no
use for, and the key -- hashFiles on dependencies.gradle -- threw the download
away on any dependency bump, which has nothing to do with what MongoDB ships.
~/.embedmongo now has its own step in the two jobs that run a server, keyed by
runner and MongoDB version and nothing else. It holds an immutable download from
fastdl.mongodb.org, the archive and the binary extracted out of it, and no
database: flapdoodle keeps data under a temp path it deletes on shutdown unless
embedded.mongodb.database-dir is set, and the only specifications that set it
point it at a @TempDir. So it belongs to no branch and survives a version bump.
The version in the key also settles the matrix: mongodb-version is ['7.0','8.0'],
and under one key the entry that missed first won and the other restored a
binary it could not use. There are no restore-keys, for the same reason.
---
.github/workflows/gradle.yml | 26 +++++++++++---------------
.github/workflows/groovy-joint-workflow.yml | 11 +++++++++--
2 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml
index 3d982359b0..6720f2590a 100644
--- a/.github/workflows/gradle.yml
+++ b/.github/workflows/gradle.yml
@@ -44,7 +44,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -94,7 +93,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -183,7 +181,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -214,7 +211,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
buildRerunTasks:
if: ${{ !contains(github.event.head_commit.message, '[skip tests]') }}
@@ -267,7 +263,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -296,7 +291,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
buildForge:
name: "Build Grails Forge (Java ${{ matrix.java }}, indy=${{ matrix.indy }})"
@@ -327,7 +321,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -448,7 +441,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -484,7 +476,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
- name: "📤 Upload Geb Reports"
if: failure()
@@ -685,10 +676,20 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
+ - name: "🗄️ Cache the mongod the embedded server runs"
+ uses: actions/cache@v4
+ with:
+ # Kept apart from the dependency jars: this holds an immutable download from
+ # fastdl.mongodb.org - the archive and the binary extracted out of it - and no database,
+ # so it belongs to no branch and does not change when a dependency version does.
+ # Keyed by the version this entry asks for, because the matrix runs more than one and a
+ # single key would keep whichever job missed first and leave the other restoring 250MB
+ # of a binary it cannot use. No restore-keys, for that same reason.
+ path: ~/.embedmongo
+ key: embedmongo-${{ runner.os }}-${{ matrix.mongodb-version }}
- name: "🐘 Setup Gradle"
uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
with:
@@ -738,7 +739,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -890,7 +890,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -958,7 +957,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -1077,7 +1075,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
@@ -1195,7 +1192,6 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
diff --git a/.github/workflows/groovy-joint-workflow.yml b/.github/workflows/groovy-joint-workflow.yml
index d9e97b9459..5aba28cd7a 100644
--- a/.github/workflows/groovy-joint-workflow.yml
+++ b/.github/workflows/groovy-joint-workflow.yml
@@ -154,10 +154,18 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
restore-keys: |
gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-
+ - name: "🗄️ Cache the mongod the embedded server runs"
+ uses: actions/cache@v4
+ with:
+ # Separate from the dependency jars, for the reasons given on the same step in
+ # gradle.yml. This job names no version, so it gets the one
+ # AbstractMongoGrailsExtension.DEFAULT_MONGO_VERSION names; a change there costs one
+ # download, not a wrong binary, since this key has no other reader.
+ path: ~/.embedmongo
+ key: embedmongo-${{ runner.os }}-7.0
- name: "🐘 Setup Gradle"
uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
with:
@@ -198,5 +206,4 @@ jobs:
path: |
~/.gradle/caches/modules-2
~/.gradle/wrapper
- ~/.embedmongo
key: gradle-deps-${{ runner.os }}-${{ github.base_ref || github.ref_name }}-${{ hashFiles('**/dependencies.gradle', '**/gradle-wrapper.properties') }}
(grails-core) 04/04: Run the isolated tests, and see their results
Posted by jd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a commit to branch revert/16192-async-changes
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit e4cd44a3f741cbd68d77b693dc09151d3b9b7b66
Author: James Daugherty <jd...@jdresources.net>
AuthorDate: Tue Aug 25 10:50:05 2026 -0400
Run the isolated tests, and see their results
Some test classes pollute static state that the rest of the fork would inherit,
so each is given a Test task of its own and excluded from `test`. A Test task
that is registered rather than the one the java plugin creates has neither
testClassesDirs nor classpath, which makes it NO-SOURCE: it reports success
having run nothing. Since `test` excludes those same classes, they ran in no
task at all. Ninety tests, across grails-test-suite-uber and
grails-test-suite-web.
This is older than it looks. b156214b99 rewrote `task x(type: Test)` as
`tasks.register(x, Test)` in November 2023, but both forms are NO-SOURCE -- the
java plugin wires source onto its own `test` task only -- so that commit
inherited the problem rather than causing it. The sharding plugin picks these
tasks up through tasks.withType(Test) and has been distributing empty tasks
across shards ever since.
Two things kept it quiet, and both are fixed here. The tasks ran nothing, and
grails-test-report matches phases by task name, so their results were invisible
to the aggregate reports even when they did run.
What ran for the first time in years mostly passed. Four did not:
RestfulControllerSpec and ResourceAnnotationRestfulControllerSpec assert that
save, update and patch render the create or edit view again for an instance with
errors, and built that instance with an empty title. Binding an empty string
stores null, and a persistent property is nullable by default in Grails 8, so
the instance was valid and the controller redirected instead. Their domains say
`title blank: false`, which meant required under the old default; it now says
`nullable: false` as well, which is what it always meant.
Also removed: tasks and patterns naming classes that no longer exist
(GrailsDomainBinderTests, ComponentValidationTests,
HibernateMappingUniqueConstraintTests, JSONBindingTests,
AutoParams*MarshallingTests, GSPResponseWriterSpec, pages/ext/jsp),
GroovyPageAttributesTests which belongs to grails-taglib, and
DefaultGrailsControllerClassSpec which belongs to grails-core. Each excluded
nothing from `test` and included nothing anywhere. testIsolatedPersistentOne
also assigned `includes` twice, so only the second ever applied.
---
grails-test-report/build.gradle | 18 ++++++++++++++++--
grails-test-suite-persistence/build.gradle | 17 +++--------------
grails-test-suite-uber/build.gradle | 6 +++++-
.../ResourceAnnotationRestfulControllerSpec.groovy | 4 +++-
.../grails/test/mixin/RestfulControllerSpec.groovy | 4 +++-
grails-test-suite-web/build.gradle | 13 ++++++++-----
6 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/grails-test-report/build.gradle b/grails-test-report/build.gradle
index 2183e8e8d9..8141660f97 100644
--- a/grails-test-report/build.gradle
+++ b/grails-test-report/build.gradle
@@ -24,12 +24,26 @@ plugins {
// the cli tier runs its tests in dedicated source sets (test-cli / integration-test-cli), so those
// phases are aggregated alongside the ordinary ones - otherwise their results would be invisible
// to the aggregate reports and to CI
+//
+// the test-suite projects also run some classes in their own Test tasks, to keep a class that
+// pollutes static state away from the rest of the fork. A phase is matched by task name, so those
+// task names have to be named here too, or their results are invisible in exactly the same way -
+// which is how they came to run nothing at all without anyone noticing.
+def isolatedUnitTestPhases = [
+ 'isolatedTestsOne',
+ 'isolatedTestsTwo',
+ 'isolatedRestRendererTests',
+ 'isolatedPersonTests',
+ 'isolatedRestfulControllerTests',
+ 'execIsolatedTests',
+]
+
def reportVariants = [
test : [
taskName : 'testAggregateTestReport',
markdownTask: 'markdownAggregateTestReport',
title : 'Grails Unit Test Report',
- phases : ['test', 'testCli'],
+ phases : ['test', 'testCli'] + isolatedUnitTestPhases,
reportDir : 'reports/tests/test',
markdownFile: 'reports/tests/test.md',
],
@@ -45,7 +59,7 @@ def reportVariants = [
taskName : 'combinedAggregateTestReport',
markdownTask: 'markdownCombinedAggregateTestReport',
title : 'Grails Combined Test Report',
- phases : ['test', 'testCli', 'integrationTest', 'integrationTestCli'],
+ phases : ['test', 'testCli', 'integrationTest', 'integrationTestCli'] + isolatedUnitTestPhases,
reportDir : 'reports/tests/combined',
markdownFile: 'reports/tests/combined.md',
],
diff --git a/grails-test-suite-persistence/build.gradle b/grails-test-suite-persistence/build.gradle
index 476fd2609b..7fa018aacf 100644
--- a/grails-test-suite-persistence/build.gradle
+++ b/grails-test-suite-persistence/build.gradle
@@ -93,22 +93,11 @@ test {
if(!isCiBuild) {
maxHeapSize = '2048m'
}
- excludes = ['**/GrailsDomainBinderTests.class',
- '**/ComponentValidationTests.class',
- '**/HibernateMappingUniqueConstraintTests.class']
}
-task testGrailsDomainBinder(type: Test) { includes = [
- '**/GrailsDomainBinderTests.class'
- ] }
-
-
-task testIsolatedPersistentOne(type: Test) {
- includes = ['**/ComponentValidationTests.class']
- includes = ['**/HibernateMappingUniqueConstraintTests.class']
-}
-
-test.dependsOn testGrailsDomainBinder, testIsolatedPersistentOne
+// GrailsDomainBinderTests, ComponentValidationTests and HibernateMappingUniqueConstraintTests were
+// each run by a Test task of their own, and each excluded from `test`. None of those three classes
+// exists any more, so the tasks and the exclusions went with them.
tasks.withType(Groovydoc).configureEach {
// tests do not have groovydoc
diff --git a/grails-test-suite-uber/build.gradle b/grails-test-suite-uber/build.gradle
index 74369b5e5b..de2aaa43b3 100644
--- a/grails-test-suite-uber/build.gradle
+++ b/grails-test-suite-uber/build.gradle
@@ -84,7 +84,6 @@ dependencies {
def isolatedTestPatterns = [
isolatedTestsOne: [
- 'org.grails.core.DefaultGrailsControllerClassSpec',
'org.grails.web.util.WebUtilsTests'
],
isolatedTestsTwo: [
@@ -118,6 +117,11 @@ def testSkippingProperties = [
isolatedTestPatterns.keySet().each { taskName ->
tasks.register(taskName, Test) {
group = 'verification'
+ // A Test task registered rather than inherited has neither of these, which makes it
+ // NO-SOURCE: it reports success having run nothing. These classes are excluded from
+ // `test` below, so without this they run in no task at all.
+ testClassesDirs = sourceSets.test.output.classesDirs
+ classpath = sourceSets.test.runtimeClasspath
filter.includePatterns = isolatedTestPatterns[taskName]
}
}
diff --git a/grails-test-suite-uber/src/test/groovy/grails/test/mixin/ResourceAnnotationRestfulControllerSpec.groovy b/grails-test-suite-uber/src/test/groovy/grails/test/mixin/ResourceAnnotationRestfulControllerSpec.groovy
index 08e310f4c6..5e407807a1 100644
--- a/grails-test-suite-uber/src/test/groovy/grails/test/mixin/ResourceAnnotationRestfulControllerSpec.groovy
+++ b/grails-test-suite-uber/src/test/groovy/grails/test/mixin/ResourceAnnotationRestfulControllerSpec.groovy
@@ -50,7 +50,9 @@ import grails.rest.*
class Video {
String title
static constraints = {
- title blank:false
+ // Required is spelled out: a persistent property is nullable by default in Grails 8, and
+ // binding an empty title stores null, which blank alone would let through.
+ title blank: false, nullable: false
}
}
''')
diff --git a/grails-test-suite-uber/src/test/groovy/grails/test/mixin/RestfulControllerSpec.groovy b/grails-test-suite-uber/src/test/groovy/grails/test/mixin/RestfulControllerSpec.groovy
index 2f1883f0db..3dc9e7190e 100644
--- a/grails-test-suite-uber/src/test/groovy/grails/test/mixin/RestfulControllerSpec.groovy
+++ b/grails-test-suite-uber/src/test/groovy/grails/test/mixin/RestfulControllerSpec.groovy
@@ -215,7 +215,9 @@ class Video {
String title
Integer numberOfMinutes
static constraints = {
- title blank:false
+ // Required is spelled out: a persistent property is nullable by default in Grails 8, and
+ // binding an empty title stores null, which blank alone would let through.
+ title blank: false, nullable: false
numberOfMinutes nullable: true
}
}
diff --git a/grails-test-suite-web/build.gradle b/grails-test-suite-web/build.gradle
index 8281c284b5..bf3cce7dba 100644
--- a/grails-test-suite-web/build.gradle
+++ b/grails-test-suite-web/build.gradle
@@ -58,25 +58,28 @@ def defaultTestConfig = {
excludes = ['**/*TestCase.class', '**/*$*.class']
}
+// Each of these pollutes static state that the rest of the fork would inherit, so they run in
+// execIsolatedTests below and are excluded from `test`. JSONBindingTests, AutoParams*MarshallingTests,
+// GSPResponseWriterSpec and everything under pages/ext/jsp no longer exist, and
+// GroovyPageAttributesTests belongs to grails-taglib, so naming them here excluded nothing.
def isolatedTests = [
'**/ContentFormatControllerTests.class',
- '**/JSONBindingTests.class',
- '**/AutoParams*MarshallingTests.class',
'**/JSONBindingToNullTests.class',
'**/ControllerWithXmlConvertersTests.class',
- '**/GroovyPageAttributesTests.class',
'**/BindingExcludeTests.class',
'**/NestedXmlBindingTests.class',
- '**/GSPResponseWriterSpec.class',
'**/RespondMethodSpec.class',
'**/ContentNegotiationSpec.class',
- '**/pages/ext/jsp/*.class',
'**/WithFormatContentTypeSpec.class',
'**/CommandObjectNoDataSpec.class'
]
tasks.register('execIsolatedTests', Test).configure { Test it ->
it.configure(defaultTestConfig)
+ // See the note in grails-test-suite-persistence: a registered Test task has no source of its
+ // own, and these classes are excluded from `test`, so without this they run nowhere.
+ it.testClassesDirs = sourceSets.test.output.classesDirs
+ it.classpath = sourceSets.test.runtimeClasspath
it.forkEvery = 1
it.includes.addAll(isolatedTests)
}
(grails-core) 01/04: Revert the async changes from #16192
Posted by jd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a commit to branch revert/16192-async-changes
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit e8f568d8f4d1c342a626fb313848b1a1b5234e79
Author: James Daugherty <jd...@jdresources.net>
AuthorDate: Tue Aug 25 10:14:09 2026 -0400
Revert the async changes from #16192
Restores AsyncWebRequestPromiseDecoratorLookupStrategy and
WebUtils.clearGrailsWebRequest to their state before #16192, and drops the
specification added with them.
The changes treated a symptom. A promise callback attached to a request the
container will not start a second asynchronous cycle on was failing the
response, and the fix caught that refusal, returned no decorators and logged at
debug. The request that reached that point is one whose data was not cleaned up
correctly; the contract now says a promise may silently run undecorated, with
nothing in the response or at any visible log level to say it happened. The
tests pass because the failure is quiet, not because the cause is gone.
The IllegalStateException swallowed in clearGrailsWebRequest is the same
symptom on the teardown side: a task cleaning up after a request the container
has already recycled.
The embedded MongoDB replica set work from #16192 is kept.
---
...WebRequestPromiseDecoratorLookupStrategy.groovy | 75 +-------------
...equestPromiseDecoratorLookupStrategySpec.groovy | 114 ---------------------
.../main/groovy/org/grails/web/util/WebUtils.java | 22 ++--
3 files changed, 7 insertions(+), 204 deletions(-)
diff --git a/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategy.groovy b/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategy.groovy
index c8e5846641..549ef40fd9 100644
--- a/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategy.groovy
+++ b/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategy.groovy
@@ -22,11 +22,6 @@ import groovy.transform.CompileStatic
import grails.async.decorator.PromiseDecorator
import grails.async.decorator.PromiseDecoratorLookupStrategy
-import groovy.util.logging.Slf4j
-import jakarta.servlet.http.HttpServletRequest
-
-import grails.async.web.AsyncGrailsWebRequest
-
import org.grails.web.servlet.mvc.GrailsWebRequest
/**
@@ -35,7 +30,6 @@ import org.grails.web.servlet.mvc.GrailsWebRequest
* @author Graeme Rocher
* @since 2.3
*/
-@Slf4j
@CompileStatic
class AsyncWebRequestPromiseDecoratorLookupStrategy implements PromiseDecoratorLookupStrategy {
@@ -44,76 +38,9 @@ class AsyncWebRequestPromiseDecoratorLookupStrategy implements PromiseDecoratorL
final webRequest = GrailsWebRequest.lookup()
if (webRequest) {
List<PromiseDecorator> decorators = []
- // Held so that a decorator which does not survive its own construction leaves the
- // request as it found it: the web request it builds stores itself on the request
- // before asking the container to start, and a later lookup that found that
- // half-built object would use one whose asynchronous context was never assigned.
- Object boundBefore = attachedWebRequest(webRequest)
- try {
- decorators.add(new AsyncWebRequestPromiseDecorator(webRequest))
- }
- catch (IllegalStateException asynchronousProcessingUnavailable) {
- reattach(webRequest, boundBefore)
- if (!supportsAsync(webRequest)) {
- // A request that cannot process asynchronously at all is a mistake worth
- // reporting: the caller asked for something this request will never do.
- throw asynchronousProcessingUnavailable
- }
- // Otherwise the request does support it and is simply past the point of taking
- // another task: the container is delivering the result of the one that ran, and
- // refuses to start a second cycle on the same request. A callback attached to a
- // promise that completed while it was being attached lands exactly here, and the
- // refusal used to escape and fail the very response being delivered.
- //
- // The refusal is caught rather than anticipated on purpose. Asking first - the
- // async manager reports a concurrent result, say - is a check the container can
- // invalidate between the answer and the call: measured over 4800 requests, asking
- // still let one through, where catching let none through over twice as many.
- log.debug('Not binding this request to the promise: {}',
- asynchronousProcessingUnavailable.message)
- return Collections.emptyList()
- }
+ decorators.add(new AsyncWebRequestPromiseDecorator(webRequest))
return decorators
}
return Collections.emptyList()
}
-
- /**
- * What the request already carries, which is either nothing or the web request of an
- * asynchronous cycle that is genuinely under way.
- */
- private static Object attachedWebRequest(GrailsWebRequest webRequest) {
- try {
- return webRequest.currentRequest.getAttribute(AsyncGrailsWebRequest.WEB_REQUEST)
- }
- catch (IllegalStateException requestIsGone) {
- return null
- }
- }
-
- private static void reattach(GrailsWebRequest webRequest, Object boundBefore) {
- try {
- HttpServletRequest request = webRequest.currentRequest
- if (boundBefore == null) {
- request.removeAttribute(AsyncGrailsWebRequest.WEB_REQUEST)
- }
- else {
- request.setAttribute(AsyncGrailsWebRequest.WEB_REQUEST, boundBefore)
- }
- }
- catch (IllegalStateException requestIsGone) {
- log.debug('The request was recycled before its binding could be put back: {}',
- requestIsGone.message)
- }
- }
-
- private static boolean supportsAsync(GrailsWebRequest webRequest) {
- try {
- return webRequest.currentRequest.asyncSupported
- }
- catch (IllegalStateException requestIsGone) {
- // Recycled, so it is in no state to take a task either way.
- return true
- }
- }
}
diff --git a/grails-async/plugin/src/test/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategySpec.groovy b/grails-async/plugin/src/test/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategySpec.groovy
deleted file mode 100644
index d699da9e0a..0000000000
--- a/grails-async/plugin/src/test/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecoratorLookupStrategySpec.groovy
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.grails.plugins.web.async
-
-import jakarta.servlet.AsyncContext
-import jakarta.servlet.ServletRequest
-import jakarta.servlet.ServletResponse
-import jakarta.servlet.http.HttpServletRequest
-
-import grails.async.web.AsyncGrailsWebRequest
-
-import org.springframework.mock.web.MockHttpServletRequest
-import org.springframework.mock.web.MockHttpServletResponse
-import org.springframework.mock.web.MockServletContext
-import org.springframework.web.context.request.RequestContextHolder
-
-import spock.lang.Specification
-
-import org.grails.web.servlet.mvc.GrailsWebRequest
-
-/**
- * What happens when a promise is handed a request the container will not start a new asynchronous
- * cycle on, which is where a callback attached to a promise that completed while it was being
- * attached arrives.
- */
-class AsyncWebRequestPromiseDecoratorLookupStrategySpec extends Specification {
-
- AsyncWebRequestPromiseDecoratorLookupStrategy strategy = new AsyncWebRequestPromiseDecoratorLookupStrategy()
-
- void cleanup() {
- RequestContextHolder.resetRequestAttributes()
- }
-
- void 'a request the container will not start again is not decorated'() {
- given: 'a request that supports asynchronous processing and refuses to start another cycle'
- bind(requestRefusingToStart())
-
- when:
- List decorators = strategy.findDecorators()
-
- then: 'the promise runs with nothing bound to it, rather than failing the response'
- decorators.isEmpty()
- }
-
- void 'a request that will not start again is left as it was found'() {
- given:
- MockHttpServletRequest request = requestRefusingToStart()
- bind(request)
-
- when: 'the decorator stores its web request on the request and is then refused'
- strategy.findDecorators()
-
- then: 'nothing half-built is left behind for the next lookup to find and use'
- request.getAttribute(AsyncGrailsWebRequest.WEB_REQUEST) == null
- }
-
- void 'a request that cannot process asynchronously at all still says so'() {
- given: 'what a caller gets for asking of a request that will never do this'
- MockHttpServletRequest request = requestRefusingToStart()
- request.asyncSupported = false
- bind(request)
-
- when:
- strategy.findDecorators()
-
- then: 'a mistake rather than a race, and reported as one'
- thrown(IllegalStateException)
- }
-
- void 'a thread with no request bound to it is not decorated'() {
- expect:
- strategy.findDecorators().isEmpty()
- }
-
- private static MockHttpServletRequest requestRefusingToStart() {
- MockHttpServletRequest request = new MockHttpServletRequest() {
-
- @Override
- AsyncContext startAsync() {
- throw new IllegalStateException(
- 'Calling [asyncStart()] is not valid for a request with Async state [DISPATCHING]')
- }
-
- @Override
- AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) {
- startAsync()
- }
- }
- request.asyncSupported = true
- request
- }
-
- private static void bind(HttpServletRequest request) {
- RequestContextHolder.setRequestAttributes(
- new GrailsWebRequest(request, new MockHttpServletResponse(), new MockServletContext()))
- }
-
-}
diff --git a/grails-web-common/src/main/groovy/org/grails/web/util/WebUtils.java b/grails-web-common/src/main/groovy/org/grails/web/util/WebUtils.java
index 4b99fbb3c0..453f0018e7 100644
--- a/grails-web-common/src/main/groovy/org/grails/web/util/WebUtils.java
+++ b/grails-web-common/src/main/groovy/org/grails/web/util/WebUtils.java
@@ -473,22 +473,12 @@ public class WebUtils extends org.springframework.web.util.WebUtils {
public static void clearGrailsWebRequest() {
RequestAttributes reqAttrs = RequestContextHolder.getRequestAttributes();
if (reqAttrs != null) {
- try {
- // First remove the web request from the HTTP request attributes. A container
- // recycles a request as soon as it completes, and an asynchronous task can be
- // cleaning up after one that has: reaching into it then throws, which used to
- // leave the thread bound and take the response down with it.
- GrailsWebRequest webRequest = (GrailsWebRequest) reqAttrs;
- webRequest.getRequest().removeAttribute(GrailsApplicationAttributes.WEB_REQUEST);
- }
- catch (IllegalStateException alreadyRecycled) {
- // the request is gone, so there is nothing left on it to remove
- }
- finally {
- // Now remove it from RequestContextHolder. This is the half that matters on a
- // pooled thread: one left carrying a finished request poisons the next it serves.
- RequestContextHolder.resetRequestAttributes();
- }
+ // First remove the web request from the HTTP request attributes.
+ GrailsWebRequest webRequest = (GrailsWebRequest) reqAttrs;
+ webRequest.getRequest().removeAttribute(GrailsApplicationAttributes.WEB_REQUEST);
+
+ // Now remove it from RequestContextHolder.
+ RequestContextHolder.resetRequestAttributes();
}
}