Skip to content

Migrate Column tests to JUnit 5 so they actually run - #273

Merged
jnioche merged 1 commit into
mainfrom
claude/gallant-neumann-644ffe
Aug 26, 2026
Merged

Migrate Column tests to JUnit 5 so they actually run#273
jnioche merged 1 commit into
mainfrom
claude/gallant-neumann-644ffe

Conversation

@jnioche

@jnioche jnioche commented Aug 26, 2026

Copy link
Copy Markdown
Member

What

ColumnTest, CURColumnTest and AzureColumnTest were written against JUnit 4 (org.junit.Test / org.junit.Assert.*), but pom.xml only declares junit-jupiter-api/engine/params — there is no junit-vintage-engine. Surefire was silently skipping all three: mvn test -Dtest=ColumnTest reported Tests run: 0, and they never showed up in the full run either.

This migrates them to JUnit 5:

  • org.junit.Testorg.junit.jupiter.api.Test
  • org.junit.Assert.*org.junit.jupiter.api.Assertions.*
  • dropped the now-redundant public modifiers on the classes and test methods

It also deletes CURColumnTest, which was a strict subset of ColumnTest — identical three methods, both constructing CURColumn, with ColumnTest adding testResolveIndexOptionalCachesMisses on top.

Result

mvn -o test: 356 → 363 tests, all green.

  • ColumnTest 4, AzureColumnTest 3 (the +3 from the deleted CURColumnTest were pure duplicates)
  • Nothing was actually broken. All ten recovered tests passed on the first run, so the production code had not drifted away from them while they were dormant.

Notes for reviewers

No assertion bodies needed changing, which is the part most likely to bite in a JUnit 4 → 5 migration:

  • None of the three files used the assertEquals(String message, ...) form, whose message argument moves to last in Jupiter. Nothing to reorder.
  • The two calls that looked at risk bind correctly anyway: assertEquals(3.14159, col.getDouble(r1), 1e-9)assertEquals(double, double, double delta), and assertEquals(-1, col.resolveIndex(missing, true))assertEquals(int, int), since Column.resolveIndex returns int. Neither silently picks up a message overload.

The diff is otherwise purely mechanical — four import/modifier changes per file.

ColumnTest, CURColumnTest and AzureColumnTest were written against
JUnit 4 (org.junit.Test / org.junit.Assert.*), but pom.xml only
declares junit-jupiter-api/engine/params with no junit-vintage-engine.
Surefire silently skipped all three: they reported "Tests run: 0" and
never appeared in the full run.

Swap the imports to org.junit.jupiter.api.Test and
org.junit.jupiter.api.Assertions.*, and drop the now-redundant public
modifiers on the classes and test methods. No assertion bodies needed
changing: none of the files used the JUnit 4
assertEquals(String message, ...) form whose message argument moves to
last in Jupiter, and the two calls that looked at risk bind correctly
anyway - assertEquals(3.14159, getDouble(r1), 1e-9) to
assertEquals(double, double, double) and assertEquals(-1,
resolveIndex(...)) to assertEquals(int, int), since resolveIndex
returns int.

All ten recovered tests passed on the first run, so nothing had drifted
while they were dormant.

Also delete CURColumnTest, which was a strict subset of ColumnTest:
identical three methods, both constructing CURColumn, with ColumnTest
adding testResolveIndexOptionalCachesMisses on top.

mvn -o test: 356 -> 363 tests, all green.
@jnioche jnioche added this to the 1.3 milestone Aug 26, 2026
@jnioche jnioche added the bug Something isn't working label Aug 26, 2026
@jnioche
jnioche merged commit fd277ff into main Aug 26, 2026
3 checks passed
@jnioche
jnioche deleted the claude/gallant-neumann-644ffe branch August 26, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant