lsttokens tests: report empty-override suppressions as skipped, not absent#7663
Open
Vest wants to merge 1 commit into
Open
lsttokens tests: report empty-override suppressions as skipped, not absent#7663Vest wants to merge 1 commit into
Vest wants to merge 1 commit into
Conversation
Concrete lst-token test classes overrode inherited @test methods with an empty body plus a code comment to mean "this scenario doesn't apply here." JUnit 5's Jupiter engine drops the parent's @test when a subclass overrides without re-declaring @test (per the docs: "Such methods are inherited unless overridden."), so the previous state was neither a passing test nor a reported skip -- the test was simply absent from the report. A reader of the subclass could not tell whether the parent's test was still running or being suppressed. Fix: re-declare @test on the override alongside @disabled("<reason>"). The reason string moves from an invisible code comment into JUnit test metadata, and each suppressed test is now reported as skipped instead of silently dropped. Files touched (11 methods, all in the lst-token test tree): AddTokenTest.java testOverwrite PreTokenTest.java testOverwrite SchoolsTokenTest.java testInvalidInputOnlySubToken testInvalidInputOnlySubTokenPipe testUnparseIllegalAllItem testUnparseIllegalAllType testUnparseIllegalItemAll testUnparseIllegalTypeAll testUnparseLegal ItypeTokenTest.java testReplacementInputs testReplacementInputsTwo (also missing @test) Verification (./gradlew :test --tests "plugin.lsttokens.*"): Before: 2399 tests, 0 skipped, 0 failures After: 2410 tests, 11 skipped, 0 failures The 11 newly-reported skips exactly match the methods annotated above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Concrete lst-token test classes overrode inherited
@Testmethods with an empty body plus a code comment to mean "this scenario doesn't apply here" — for example:The intent was to suppress the parent's test. What actually happens under JUnit 5 is subtly worse than expected: Jupiter drops the parent's
@Testwhen the subclass overrides it without re-declaring@Test(docs: "Such methods are inherited unless overridden."). So the previous state was neither a passing test nor a reported skip — the test was simply absent from the report. A reader of the subclass could not tell whether the parent's test was still running or being suppressed.Fix
Re-declare
@Teston the override alongside@Disabled("<reason>"). The reason string moves out of an invisible code comment into JUnit test metadata, and each suppressed test is now reported as skipped with its rationale instead of silently dropped.Methods touched (11 total)
AddTokenTesttestOverwritePreTokenTesttestOverwriteSchoolsTokenTesttestInvalidInputOnlySubToken,testInvalidInputOnlySubTokenPipeSchoolsTokenTesttestUnparseIllegalAllItem,…AllType,…ItemAll,…TypeAll,…LegalItypeTokenTesttestReplacementInputs,testReplacementInputsTwoItypeTokenTest.testReplacementInputsTwowas carrying the same anti-pattern without any comment at all — folded in here because the fix is identical.Verification
./gradlew :test --tests "plugin.lsttokens.*"(aggregated across all suites in the package tree):The 11 newly-reported skips exactly match the 11 methods annotated above — no other tests were affected.
Diff stats
Pure test-metadata cleanup. No production code changed.