Skip to content

Migrate the test suite to JUnit Jupiter - #415

Merged
jmanico merged 5 commits into
mainfrom
junit5-migration
Sep 8, 2026
Merged

jmanico merged 5 commits into
mainfrom
junit5-migration

Conversation

@jmanico

@jmanico jmanico commented Sep 8, 2026 •

Copy link
Copy Markdown
Member

Completes #374 by @strangelookingnerd on top of current main. That branch had drifted far behind (every test file changed on both sides, and ElidedCharactersTest was added after it was opened), so the migration was redone from scratch with the same goals and the original author credited as co-author. A full multi-angle review then ran against the result; everything it found is fixed in the later commits.

What changes

  • JUnit Jupiter 5.14.4 via the JUnit BOM. The 5.x line is the last one that supports the Java 8 target; JUnit 6 requires Java 17. java8-shim and java10-shim have no tests and drop their JUnit dependency.
  • Every JUnit 3 test* method is now @Test, package-private and non-static. JUnit 3's reflection runner found public static final void test*() methods; Jupiter silently ignores static ones, so this was the main thing to get right.
  • Message-first assertions reordered to Jupiter's (expected, actual, message) form (65 call sites, done with a paren-aware parser and independently re-verified by the review).
  • setUp/tearDown overrides become named @BeforeEach/@AfterEach methods; AntiSamyTest.suite() and the JUnit 3 VerboseTestRunner are removed.
  • Expected exceptions use assertThrows; error lists are compared with assertIterableEquals; ExamplesTest runs each example as its own case via @FieldSource and rethrows the example's real exception.
  • Single-method anonymous Handler / AttributePolicy / ElementPolicy / HtmlStreamEventProcessor / Function / Runnable classes are lambdas; stray tab indentation is normalized.

Strengthened by the review

  • Fuzzers: FuzzyTestCase registers a TestWatcher that prints replay it with -Djunit.seed=<seed> on any failure, and rejects a malformed seed instead of silently falling back to the clock. The sanitizer fuzzer's worker now catches Throwable, so an AssertionError or stack overflow in a pool thread fails the test instead of vanishing. CssFuzzerTest's slow-input watchdog, which could never fire, works.
  • Builder tests: HtmlPolicyBuilderTest.apply used to pass its failing handler as an unused context object, so renderer errors never failed any of ~40 tests. It now renders through a real failing handler, using a new package-private PolicyFactory.preprocessor() accessor so the preprocessor pipeline is unchanged. This is the only production-source change, and it adds no public API.
  • Tag balancing: the nested-anchor test hidden behind a failingtest name prefix expected malformed HTML. Its golden is corrected to the well-formed output the balancer actually produces, and it is enabled and passing.

Verification

main this branch
tests run 378 381
failures / errors 0 / 0 0 / 0
skipped 0 0

Per-class counts are identical to main; the +3 are the two extra parameterized example cases and the newly enabled nested-anchor test. ./mvnw verify passes on JDK 11, 17, 21 and 25 in CI and locally.

Closes #374

🤖 Generated with Claude Code

https://claude.ai/code/session_01BZfawfTUAFHN7cocUjcpQK

The tests were JUnit 3 `TestCase` subclasses run through the JUnit 4
provider, with `public static final void test*()` methods that only the
JUnit 3 reflection runner would find.  They now run on JUnit Jupiter 5.14
(the last line that still supports the Java 8 target) via the JUnit BOM.

* Every `test*` method carries `@Test` and is package-private and
  non-static; JUnit 3/4 message-first assertions are reordered to the
  Jupiter `(expected, actual, message)` form.
* `setUp`/`tearDown` overrides become named `@BeforeEach`/`@AfterEach`
  methods; the AntiSamy `suite()` and the JUnit 3 `VerboseTestRunner`
  are gone.
* Expected exceptions use `assertThrows`; the example programs run as a
  `@ParameterizedTest`, one case per example, and the diagnostic output
  of a failing example now reaches the real stderr.
* The nested-anchor tag balancing test that was hidden behind a
  `failingtest` name prefix is `@Disabled` with a reason, so it shows up
  as skipped instead of vanishing.
* Single-method anonymous `Handler`, `AttributePolicy`, `ElementPolicy`,
  `Function` and `Runnable` classes are lambdas; stray tab indentation
  in four test files is normalized.
* `java8-shim` and `java10-shim` have no tests and drop their JUnit
  dependency.

Per-class test counts are unchanged from main (378), plus the two extra
example cases and the newly visible skipped test.  `./mvnw verify`
passes on JDK 11, 17, 21 and 25, and `-Djunit.seed=<seed>` still
replays the fuzzers.

Supersedes #374.

Co-authored-by: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZfawfTUAFHN7cocUjcpQK
Reviewing #374 against #415 turned up cleanups the contributor made
that the fresh migration lacked: String.join in place of stream/collect
joins, assertArrayEquals for the code unit comparison, lambdas for the
preprocessor wrappers and the benchmark's error handlers, an enhanced
for loop with StandardCharsets over the AntiSamy base64 inputs,
StringBuilder for StringBuffer, and redundant `final` dropped from
private static methods.

Co-authored-by: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZfawfTUAFHN7cocUjcpQK
* HtmlPolicyBuilderTest.apply passed its failing Handler as the unused
  context argument of PolicyFactory.sanitize, so renderer errors never
  failed any builder test.  It now renders through a handler that fails
  the test, applying the factory's preprocessor via a new package-private
  PolicyFactory.preprocessor() accessor so behaviour is otherwise
  unchanged.
* ExamplesTest rethrows the example's own exception instead of the
  reflective InvocationTargetException wrapper, and takes its cases
  straight from AllExamples.CLASSES with @FieldSource.
* The postprocessor wrappers in HtmlPolicyBuilderTest use the same lambda
  form as the preprocessor ones; a `throws Exception` clause that the
  StandardCharsets change made dead and the last Eclipse-only "javadoc"
  suppression token are gone.
* The @disabled reason on the nested-anchor balancing test notes that
  its expected output also needs settling.
* java8-shim/pom.xml drops the empty <dependencies> element.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZfawfTUAFHN7cocUjcpQK
jmanico and others added 2 commits September 7, 2026 18:58
The worker task only caught Exception, so an AssertionError from the
lexer or a StackOverflowError in a pool thread went to the default
uncaught-exception handler and the test still passed without printing
the seed.  Catch Throwable, and rethrow Errors as themselves.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZfawfTUAFHN7cocUjcpQK
* FuzzyTestCase seeds a final Random in its field initializer, rejects a
  malformed junit.seed instead of silently using the clock, and registers
  a TestWatcher that prints the seed of any failed fuzz test.  The three
  fuzzers drop their ad-hoc seed printing (which appended an "L" that the
  parser then refused) and become package-private like the other tests.
* CssFuzzerTest's slow-input watchdog could never fire: it looped while
  the input was null and was started only after the input was set.  It
  now runs for the whole test, reports each slow input once, and is
  interrupted at the end.
* The nested-anchor tag balancing test is enabled with the well-formed
  output the balancer actually produces; its golden closed an <a> that
  was never opened.
* Error lists are compared as lists with assertIterableEquals rather than
  as newline-joined strings, and CssTokensTest drops its redundant
  toString pre-checks.
* HtmlStreamRendererTest loses a @beforeeach that reset per-instance
  fields, AntiSamyTest uses Handler.PROPAGATE, and the tag balancing
  handler includes the renderer's message in its failure.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZfawfTUAFHN7cocUjcpQK
@jmanico
jmanico merged commit b7edd9f into main Sep 8, 2026
7 checks passed
@jmanico
jmanico deleted the junit5-migration branch September 11, 2026 01:37
@github-actions github-actions Bot added the released Issue has been released label Sep 21, 2026
@github-actions

Copy link
Copy Markdown

🎉 This issue has been resolved in release-20260921.1 (Release Notes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Issue has been released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant