Skip to content

lsttokens tests: report empty-override suppressions as skipped, not absent#7663

Open
Vest wants to merge 1 commit into
PCGen:masterfrom
Vest:disabled-empty-override-tests
Open

lsttokens tests: report empty-override suppressions as skipped, not absent#7663
Vest wants to merge 1 commit into
PCGen:masterfrom
Vest:disabled-empty-override-tests

Conversation

@Vest

@Vest Vest commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

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" — for example:

@Override
public void testOverwrite()
{
    // Can't be done, nothing ever unparses
}

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 @Test when 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 @Test on 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)

File Method(s) Reason
AddTokenTest testOverwrite "Can't be done, nothing ever unparses"
PreTokenTest testOverwrite "Can't be done, nothing ever unparses"
SchoolsTokenTest testInvalidInputOnlySubToken, testInvalidInputOnlySubTokenPipe "Must ignore due to 5.16 syntax"
SchoolsTokenTest testUnparseIllegalAllItem, …AllType, …ItemAll, …TypeAll, …Legal "SpellSchool doesn't have a RM"
ItypeTokenTest testReplacementInputs, testReplacementInputsTwo "514 behavior, to be changed after 5.16?"

ItypeTokenTest.testReplacementInputsTwo was 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):

Before After
tests 2399 2410
skipped 0 11
failures 0 0
errors 0 0

The 11 newly-reported skips exactly match the 11 methods annotated above — no other tests were affected.

Diff stats

 4 files changed, 26 insertions(+), 10 deletions(-)

Pure test-metadata cleanup. No production code changed.



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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant