search

junit-team / junit4

A programmer-oriented testing framework for Java.
https://junit.org/junit4
Eclipse Public License 1.0
8.51k stars 3.24k forks source link

Treat org.opentest4j.TestAbortedException the same as AssumptionViolatedException #1768

Closed vlsi closed 6 months ago

vlsi commented 8 months ago

Imagine the project migrates from JUnit4 to JUnit5. They might have a helper method executing assumeTrue. However, if the project migrates to org.junit.jupiter.api .Assumptions.assumeTrue the error flips to org.opentest4j.TestAbortedException so the test no longer works. I guess the reason is junit4 does not recognize TestAbortedException and it treats the error as the regular test failure.

I am running junit:junit:4.13.2 test with org.junit.vintage:junit-vintage-engine:5.9.3

Here's a sample failure:

https://github.com/pgjdbc/pgjdbc/actions/runs/6795875259/job/18474816613?pr=2979#step:8:450

    org.opentest4j.TestAbortedException: Assumption failed: assumption is not true
        at app//org.junit.jupiter.api.Assumptions.throwAssumptionFailed(Assumptions.java:316)
        at app//org.junit.jupiter.api.Assumptions.assumeTrue(Assumptions.java:115)
        at app//org.junit.jupiter.api.Assumptions.assumeTrue(Assumptions.java:66)
        at app//org.postgresql.test.TestUtil.assumeHaveMinimumServerVersion(TestUtil.java:791)
        at app//org.postgresql.test.jdbc2.RefCursorFetchTest.beforeClass(RefCursorFetchTest.java:79)

FAILURE   0,0sec,    1 completed,   1 failed,   0 skipped, org.postgresql.test.jdbc2.RefCursorFetchTest
kcooney commented 7 months ago

JUnit4 is in maintenance mode, so it is hard to predict when or if this might be addressed.

According to the Migrating from JUnit 4 JUnit Jupiter supports AssumptionViolatedException; have you tried having assumeHaveMinimumServerVersion() use org.junit.Assume?

vlsi commented 7 months ago

@kcooney , thanks for the comment, however, I want to migrate to JUnit5 fully, so I would like to refrain from mixing different Assume calls in the test code.

kcooney commented 7 months ago

@kcooney , thanks for the comment, however, I want to migrate to JUnit5 fully, so I would like to refrain from mixing different Assume calls in the test code.

Couldn't you have the Assume calls in your shared test code be the last code you migrate? It sounds like this issue only affects you mid-migration.

Note that multiple runners and rules have logic for AssumptionViolatedException so even if the core JUnit code was updated to support TestAbortedException we could not guarantee that third party rules or runners would work.

vlsi commented 6 months ago

I went with excluding TestUtil from openrewrite processing (so it will use JUnit4's Assumption), and I will remove the exclusions as I manually migrate the remaining JUnit4 tests to JUnit5. I guess this can be closed as I do not need the change anymore.