Leonard Brünings created GROOVY-12145:
-----------------------------------------
Summary: JUnit 5 migration silently disabled ~195 test methods:
base class removed but @Test not added
Key: GROOVY-12145
URL: https://issues.apache.org/jira/browse/GROOVY-12145
Project: Groovy
Issue Type: Bug
Reporter: Leonard Brünings
h2. Summary
The JUnit 5 migration commit {{a1897e0811}} ("Convert to JUnit5", Daniel Sun)
converted many JUnit3-style Groovy tests ({{extends GroovyTestCase}}, methods
named {{void testXxx()}}) to JUnit 5. The mechanical part of the conversion was
to (a) drop the {{GroovyTestCase}} base class and (b) add {{@Test}} to every
{{void testXxx()}} method.
For a number of files step (b) was *partially or completely skipped*. Because
the build uses {{useJUnitPlatform()}} and the JUnit Platform discovers tests
*only* by the {{@Test}} annotation (there is no longer a {{TestCase}} base
class for the vintage engine to discover), every un-annotated {{void
testXxx()}} method *silently stopped executing*. The build stays green because
"0 tests" is not a failure.
*Headline numbers* (see the Totals table for the full breakdown, and
"Verification and confidence" for how each portion was established):
* *~195 dead test methods across 32 files* total, spanning {{a1897e0811}} and
its sibling JUnit 5 migration commits.
* *~175 of those (27 files) are attributable to {{a1897e0811}} alone*; the
remaining ~20 (5 files) come from sibling migration commits and are listed
under category (c) for completeness.
* *~169 methods across 19 classes are fully dead and were confirmed by
execution* (the class discovers *0 tests*). The remaining *~26 partially-dead
methods are source-inferred only* (method present, annotation absent) and were
*not* individually confirmed by a per-method execution diff.
This is the same defect already known for {{groovy-console
AstNodeToScriptAdapterTest}} (63 methods) — see "Already tracked" below. That
file is *not* included in the counts here because it is being fixed under a
separate change.
h2. Root cause
* {{extends GroovyTestCase}} was removed from the class.
* {{@Test}} was *not* added (or was added to only some methods).
* No {{@RunWith}} / JUnit4 style and no {{Specification}} base remain.
* Result: JUnit Platform discovers *nothing* (fully-dead file) or *only the
annotated subset* (partially-dead file). The remaining {{testXxx()}} methods
never run.
A secondary variant of the same commit: in several bug tests the injected
{{import org.junit.jupiter.api.Test}} was placed *inside a triple-quoted script
string* (e.g. inside {{shell.evaluate """ ... """}}) instead of at the top of
the file — so the import is dead code inside test data and no annotation was
ever added.
A third, *collateral* variant: {{a1897e0811}} removed {{extends
GroovyTestCase}} from the *abstract base* {{BuilderTestSupport}}. Its two
concrete subclasses were never edited, but they inherited neither {{@Test}} nor
a discovering base, so they died too.
h2. Reproduction
Any listed fully-dead class produces "No tests found", e.g. (verified
empirically):
{code}
./gradlew :groovy-astbuilder:test --tests
'org.codehaus.groovy.ast.builder.AstBuilderFromSpecificationTest'
> Task :groovy-astbuilder:test FAILED
Gradle Test Run :groovy-astbuilder:test: 0 passed, 0 skipped (0 tests)
> No tests found for given includes:
[org.codehaus.groovy.ast.builder.AstBuilderFromSpecificationTest](--tests
filter)
{code}
For a file that still {{extends groovy.test.GroovyTestCase}} the vintage engine
runs it normally — those files are *fine* and are excluded from this report.
h2. Verification and confidence
*This section is authoritative.* Where the per-file tables below (which were
derived statically from source) disagree with what is stated here, the
executed/verified facts here win.
h3. Fully-dead classes — verified by execution
Every *fully-dead* class was confirmed by running it through a {{--tests}}
filter and observing {{0 tests}} / "No tests found" — not merely inferred from
source. Use the *fully-qualified* class name; a {{*Wildcard}} pattern does not
filter reliably in this build and silently runs a broad set instead.
*Confirmed dead (0 tests discovered):*
* Core {{:test}} (13):
{{org.codehaus.groovy.runtime.trampoline.TrampolineTest}},
{{bugs.Groovy779_Bug}}, {{groovy.util.ResourceBundleTest}},
{{org.codehaus.groovy.tools.rootloadersync.SubclassingInGroovyTest}},
{{org.codehaus.groovy.runtime.memoize.CacheCleanupTest}},
{{bugs.Groovy4414Bug}}, {{groovy.SingletonBugTest}}, {{groovy.StackTraceTest}},
{{bugs.Groovy7031Bug}}, {{bugs.Groovy8212Bug}}, {{bugs.Groovy4080Bug}},
{{bugs.Groovy6755Bug}}, {{bugs.Groovy8138Bug}}
* {{:groovy-astbuilder}} (1):
{{org.codehaus.groovy.ast.builder.AstBuilderFromSpecificationTest}}
* {{:groovy-xml}} (2, collateral via {{BuilderTestSupport}}):
{{groovy.xml.MarkupBuilderTest}}, {{groovy.xml.StreamingMarkupBuilderTest}}
* {{:groovy-templates}} (1): {{groovy.bugs.groovy4585.Groovy4585Bug}}
* {{:groovy-json}} (1): {{json.JsonTest}}
* {{:groovy-contracts}} (1):
{{org.apache.groovy.contracts.tests.post.ImplicitVariableNamesPostconditionTests}}
That is *19 classes verified by execution* (~169 dead test methods). These map
onto the tables below as: category (a) minus its abstract base (15 classes),
category (a2) (2 classes), and the two *fully-dead* rows of category (c) —
{{json.JsonTest}} and {{ImplicitVariableNamesPostconditionTests}}. The abstract
base {{BuilderTestSupport}} cannot be run directly; its deadness is *inferred*
but demonstrated in practice by its two concrete subclasses above, which do run
and discover 0 tests.
h3. Partially-dead classes — source-inferred only
The per-file counts for the *partially-dead* tables ((b) and the partial rows
of (c)) were derived *statically* and are *not* each confirmed by a per-method
execution diff. Two caveats apply:
* *A gross execution count cannot prove a specific named method is dead.* The
partial-dead claims rest on source inspection (method present,
{{@Test}}/{{@ParameterizedTest}} annotation absent) — sound, but not
execution-confirmed per method.
* *The static counts do not account for {{@ParameterizedTest}} methods*, so the
number of executions a class reports can *exceed* its {{@Test}} count.
Spot-checks: {{groovy.DynamicMemberTest}} ran 1 (matches its table),
{{SortableTransformTest}} ran 11 (matches), but {{groovy.ForLoopTest}} ran *26*
— parameterized inflation well above its tabulated 18 {{@Test}}. Its named
victim {{testClassicForWithMultiAssignment2}} nonetheless carries no
{{@Test}}/{{@ParameterizedTest}} and so is genuinely undiscoverable. Read the
"# test methods" / "# @Test" columns below as *source counts*, not as execution
counts.
h2. (a) Fully-dead files caused by {{a1897e0811}}
*Verified by execution* (each concrete class discovers 0 tests), except the
abstract base {{BuilderTestSupport}}, whose deadness is inferred via its
subclasses in (a2). Base class removed, *zero* {{@Test}} annotations added —
every method below never runs.
||File||# test methods||# @Test||stray import-in-string||
|{{subprojects/groovy-astbuilder/.../AstBuilderFromSpecificationTest.groovy}}|67|0|no|
|{{subprojects/groovy-xml/.../groovy/xml/BuilderTestSupport.groovy}} (abstract
base; inferred, see collateral below)|15|0|no|
|{{src/test/groovy/org/codehaus/groovy/runtime/trampoline/TrampolineTest.groovy}}|4|0|no|
|{{src/test/groovy/bugs/Groovy779_Bug.groovy}}|3|0|no|
|{{src/test/groovy/groovy/util/ResourceBundleTest.groovy}}|3|0|no|
|{{src/test/groovy/org/codehaus/groovy/tools/rootloadersync/SubclassingInGroovyTest.groovy}}|2|0|no|
|{{src/test/groovy/org/codehaus/groovy/runtime/memoize/CacheCleanupTest.groovy}}|2|0|no|
|{{src/test/groovy/bugs/Groovy4414Bug.groovy}}|2|0|no|
|{{src/test/groovy/groovy/SingletonBugTest.groovy}}|2|0|no|
|{{src/test/groovy/groovy/StackTraceTest.groovy}}|2|0|no|
|{{subprojects/groovy-templates/.../groovy/bugs/groovy4585/Groovy4585Bug.groovy}}|1|0|no|
|{{src/test/groovy/bugs/Groovy7031Bug.groovy}}|1|0|no|
|{{src/test/groovy/bugs/Groovy8212Bug.groovy}}|1|0|no|
|{{src/test/groovy/bugs/Groovy4080Bug.groovy}}|1|0|*YES*|
|{{src/test/groovy/bugs/Groovy6755Bug.groovy}}|1|0|*YES*|
|{{src/test/groovy/bugs/Groovy8138Bug.groovy}}|1|0|*YES*|
*Subtotal: 16 files, 108 dead test methods* (3 with the stray import-in-string
bug). Of these, 15 concrete classes are execution-confirmed; the abstract base
{{BuilderTestSupport}} is inferred.
h3. (a2) Collateral fully-dead files (base {{BuilderTestSupport}} was migrated)
*Verified by execution.* These two files were *not* edited by {{a1897e0811}},
but they {{extends BuilderTestSupport}}, whose {{GroovyTestCase}} base was
removed. They have no {{@Test}} of their own, so all inherited + local tests
are dead.
||File||# test methods (own + inherited)||# @Test||
|{{subprojects/groovy-xml/.../groovy/xml/MarkupBuilderTest.groovy}}|29 (14 own
+ 15 inherited)|0|
|{{subprojects/groovy-xml/.../groovy/xml/StreamingMarkupBuilderTest.groovy}}|18
(3 own + 15 inherited)|0|
*Subtotal: 2 files, 47 dead test executions.*
h2. (b) Partially-dead files caused by {{a1897e0811}}
*Source-inferred only* (see "Partially-dead classes" above): the counts are
static source counts and are not each confirmed by a per-method execution diff.
h3. (b1) Base class removed, only *some* {{@Test}} added
||File||# test methods||# @Test||# dead||dead methods||
|{{src/test/groovy/groovy/DynamicMemberTest.groovy}}|6|1|5|{{testGStringMethodCall}},
{{testGStringPropertyAccess}}, {{testStringMethodCallAndAttributeAccess}},
{{testDynamicAttributeAccess}}, {{testDynamicMethodClosure}}|
|{{src/test/groovy/groovy/lang/ScriptCacheTest.groovy}}|2|1|1|{{testScriptCaching}}|
h3. (b2) Touched by {{a1897e0811}}, single/few {{testXxx()}} left un-annotated
The bulk of these classes are annotated, but the listed methods were
overlooked. Verified as *standalone* tests (not helpers — each is referenced
only by its own declaration).
||File||# test methods||# @Test||# dead||dead methods||
|{{subprojects/groovy-json/.../groovy/json/JsonSlurperLaxTest.groovy}}|8 own
(+60 inherited @Test)|60 inherited|8|{{testNullEmptyMalformedPayloads}},
{{testObjectWithSimpleValues}}, {{testArrayOfArrayWithSimpleValues}},
{{testBackSlashEscaping}}, {{testLaxCommentsAndKeys}}, {{testInvalidNumbers}}*,
{{testGroovy7728}}, {{testGroovy9954}}|
|{{subprojects/groovy-json/.../groovy/json/JsonSlurperCharSourceTest.groovy}}|1
own (+60 inherited @Test)|60 inherited|1|{{testParsingDecimalValue}}|
|{{src/test/groovy/gls/annotations/AnnotationTest.groovy}}|64|63|1|{{testAnnotateAnnotationDefinitionWithAnnotationWithAnnotationTypeTarget}}|
|{{src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy}}|7|6|1|{{testFlowTyping4}}|
|{{src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java}}|6|5|1|{{testSetViaSystemProperties}}|
|{{src/test/groovy/bugs/Groovy11046.groovy}}|2|1|1|{{testMissingDependency2}}|
|{{subprojects/groovy-contracts/.../tests/doc/RootClassExampleTests.groovy}}|18|17|1|{{test_postcond_with_result_and_old_variables_switched}}|
{{*}} {{JsonSlurperLaxTest.testInvalidNumbers}} is an {{@Override}} of a
{{@Test}} method in the parent {{JsonSlurperTest}}; whether it executes depends
on Jupiter's inherited-annotation resolution — treat as suspect. The other 7
Lax-specific methods are unambiguously dead. ({{JsonSlurperLaxTest}} /
{{JsonSlurperCharSourceTest}} still run the 60 inherited parent tests, so only
the subclass-specific methods are lost.)
*Subtotal (b): 9 files, 20 dead test methods (source-inferred).*
h2. (c) Related — same defect, introduced by sibling JUnit 5 migration commits
(not {{a1897e0811}})
Found during the same sweep. Same root cause, different commit; listed for
completeness. The two *fully dead* rows ({{JsonTest}},
{{ImplicitVariableNamesPostconditionTests}}) are *execution-verified* (see
above); the three partial rows are *source-inferred only* and are subject to
the {{@ParameterizedTest}} caveat (notably {{ForLoopTest}} actually executes
26, above its tabulated 18 {{@Test}}).
||File||# test methods||# @Test||# dead||verification||migrating commit /
cause||
|{{subprojects/groovy-json/src/spec/test/json/JsonTest.groovy}}|13|0|13 (fully
dead)|execution-verified|{{92cbb3f39c}} "minor refactor: junit5" — base
removed, no {{@Test}}|
|{{src/test/groovy/org/codehaus/groovy/transform/SortableTransformTest.groovy}}|15|11|4|source-inferred|{{5afd9b9a91}}
"junit jupiter" removed {{GroovyTestCase}} from base {{CompilableTestSupport}}|
|{{src/test/groovy/gls/invocation/MethodSelectionTest.groovy}}|18|17|1|source-inferred|same
— base {{CompilableTestSupport}}|
|{{subprojects/groovy-contracts/.../tests/post/ImplicitVariableNamesPostconditionTests.groovy}}|1|0|1
(fully dead)|execution-verified|{{extends BaseTestClass}} (no {{@Test}}, no
discovering base); pre-existing / not {{a1897e0811}}|
|{{src/test/groovy/groovy/ForLoopTest.groovy}}|20|18|1
({{testClassicForWithMultiAssignment2}})|source-inferred (class runs 26 via
{{@ParameterizedTest}}; named victim still un-annotated)|migrated by an earlier
commit; single missed {{@Test}}|
*Subtotal (c): 5 files, 20 dead test methods* (2 files / 14 methods
execution-verified fully dead; 3 files / 6 methods source-inferred partial).
h2. Totals
The "# Dead test methods" figures are the source-derived method counts; the
"Verified by" column records how the deadness was established (execution vs.
source inspection), per the "Verification and confidence" section.
||Category||Files||Dead test methods||Verified by||
|(a) Fully-dead, {{a1897e0811}} base removed|16|108|execution (15 concrete
classes) + inference for abstract base {{BuilderTestSupport}}|
|(a2) Collateral fully-dead ({{BuilderTestSupport}} subclasses)|2|47|execution|
|(b) Partially-dead, {{a1897e0811}}|9|20|source inspection only|
|*{{a1897e0811}} total*|*27*|*~175*| |
|(c) Related (sibling migration commits)|5|20|execution (2 fully-dead classes,
14 methods) + source inspection (3 partial classes, 6 methods)|
|*Grand total*|*32*|*~195*| |
*Verified-vs-inferred split of the grand total:* ~169 methods across *19
fully-dead classes* are confirmed by execution; the remaining ~26 methods
(categories (b) and the partial rows of (c)) are *source-inferred only*.
h2. Suggested fix
* For every file above, add {{@Test}} (import {{org.junit.jupiter.api.Test}})
to each standalone {{void testXxx()}} method that should run.
* For the *stray import-in-string* files ({{Groovy4080Bug}}, {{Groovy6755Bug}},
{{Groovy8138Bug}}) — and the already-tracked {{AstNodeToScriptAdapterTest}} —
move {{import org.junit.jupiter.api.Test}} out of the triple-quoted script
string to the top-level import block, then annotate the methods.
* For the {{BuilderTestSupport}} / {{CompilableTestSupport}} families: either
annotate all inherited {{testXxx()}} in the concrete subclasses
({{MarkupBuilderTest}}, {{StreamingMarkupBuilderTest}},
{{SortableTransformTest}}, {{MethodSelectionTest}}), or restore a
JUnit-discoverable arrangement.
* Consider a CI guard: fail the build (or a lint check) when a test *class
file* named {{*Test}}/{{*Bug}} contributes *0* discovered tests, to prevent
silent regressions of this kind.
h2. Already tracked / excluded
*
{{subprojects/groovy-console/.../groovy/console/ui/AstNodeToScriptAdapterTest.groovy}}
(63 dead methods + stray import-in-string in
{{testAtImmutableClassWithProperties}}) is the original signature of this bug
and is *already being fixed under a separate change* GROOVY-12144 —
cross-reference only, not counted above.
h3. Method notes (how the list was produced)
* Started from the {{git show --name-only a1897e0811}} file set (1225 source
files).
* Parsed each file with string/comment masking so that {{void testXxx()}} and
{{@Test}} occurrences *inside triple-quoted script strings* are ignored (this
removed many false positives such as {{Groovy10133}} and
{{TraitASTTransformationTest}}, whose "missing" annotations were embedded test
scripts / a custom {{@CompileModesTest}} = {{@ParameterizedTest}} collector).
* Recognized custom test meta-annotations (e.g. {{@ParameterizedTest}}
collectors) and resolved class-inheritance chains so subclasses of a
still-{{GroovyTestCase}} base (e.g. {{StaticImportTest extends
groovy.test.GroovyTestCase}}) are correctly treated as *fine*.
* Excluded parameterized/helper methods (those with a parameter list, e.g.
{{AuthorTestHelper.testAuthorDataSet(Sql)}}) and private methods.
* Confirmed attribution with {{git show a1897e0811 -- <file>}} (base-class
removal + net {{@Test}} added).
* *Fully-dead classes were then confirmed empirically* via Gradle ({{--tests}}
filter → "No tests found"); *partially-dead classes were left at
source-inferred confidence* because a gross execution count (inflated by
{{@ParameterizedTest}}) cannot prove a specific named method is dead — see
"Verification and confidence".
{panel:title=Provenance}
Assisted-by: Claude Code (Opus 4.8).
{panel}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
