Skip to content

Commit 1d843ec

Browse files
committed
Run the isolated tests in the uber suite
Some classes here 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, all forty ran in no task at all. This is older than it looks. b156214 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.
1 parent 592b38f commit 1d843ec

4 files changed

Lines changed: 22 additions & 4 deletions

File tree

grails-test-report/build.gradle

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,25 @@ plugins {
2424
// the cli tier runs its tests in dedicated source sets (test-cli / integration-test-cli), so those
2525
// phases are aggregated alongside the ordinary ones - otherwise their results would be invisible
2626
// to the aggregate reports and to CI
27+
//
28+
// the test-suite projects also run some classes in their own Test tasks, to keep a class that
29+
// pollutes static state away from the rest of the fork. A phase is matched by task name, so those
30+
// task names have to be named here too, or their results are invisible in exactly the same way -
31+
// which is how they came to run nothing at all without anyone noticing.
32+
def isolatedUnitTestPhases = [
33+
'isolatedTestsOne',
34+
'isolatedTestsTwo',
35+
'isolatedRestRendererTests',
36+
'isolatedPersonTests',
37+
'isolatedRestfulControllerTests',
38+
]
39+
2740
def reportVariants = [
2841
test : [
2942
taskName : 'testAggregateTestReport',
3043
markdownTask: 'markdownAggregateTestReport',
3144
title : 'Grails Unit Test Report',
32-
phases : ['test', 'testCli'],
45+
phases : ['test', 'testCli'] + isolatedUnitTestPhases,
3346
reportDir : 'reports/tests/test',
3447
markdownFile: 'reports/tests/test.md',
3548
],
@@ -45,7 +58,7 @@ def reportVariants = [
4558
taskName : 'combinedAggregateTestReport',
4659
markdownTask: 'markdownCombinedAggregateTestReport',
4760
title : 'Grails Combined Test Report',
48-
phases : ['test', 'testCli', 'integrationTest', 'integrationTestCli'],
61+
phases : ['test', 'testCli', 'integrationTest', 'integrationTestCli'] + isolatedUnitTestPhases,
4962
reportDir : 'reports/tests/combined',
5063
markdownFile: 'reports/tests/combined.md',
5164
],

grails-test-suite-uber/build.gradle

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ def testSkippingProperties = [
118118
isolatedTestPatterns.keySet().each { taskName ->
119119
tasks.register(taskName, Test) {
120120
group = 'verification'
121+
// A Test task registered rather than inherited has neither of these, which makes it
122+
// NO-SOURCE: it reports success having run nothing. These classes are excluded from
123+
// `test` below, so without this they run in no task at all.
124+
testClassesDirs = sourceSets.test.output.classesDirs
125+
classpath = sourceSets.test.runtimeClasspath
121126
filter.includePatterns = isolatedTestPatterns[taskName]
122127
}
123128
}

grails-test-suite-uber/src/test/groovy/grails/test/mixin/ResourceAnnotationRestfulControllerSpec.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import grails.rest.*
5050
class Video {
5151
String title
5252
static constraints = {
53-
title blank:false
53+
title blank: false, nullable: false
5454
}
5555
}
5656
''')

grails-test-suite-uber/src/test/groovy/grails/test/mixin/RestfulControllerSpec.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ class Video {
215215
String title
216216
Integer numberOfMinutes
217217
static constraints = {
218-
title blank:false
218+
title blank: false, nullable: false
219219
numberOfMinutes nullable: true
220220
}
221221
}

0 commit comments

Comments
 (0)