Skip to content

Commit

Permalink
[OX-1076] JUnit5: test should fail on container level exception (#32)
Browse files Browse the repository at this point in the history
It has been observed in AMP that JUnit5 tests can fail silently (no test run) when 
* exception thrown in `@BeforeAll` functions
* exception thrown in private member inline initialization
* missing dependency in BUCK config

The root cause is the `Junit5TestListener` skips test identifiers of type `org.junit.platform.engine.Type.CONTAINER` which results in exceptions not reported when they are not thrown by `@Test` annotated functions.

The PR
* removes test identifier type check so that container level failures are reported.
* adds two junit libs so that junit 5 test can run in the repo.
* uses test's LegacyReportingName so that parameterized test name can be displayed properly
  * example displayName: `[1] 1`
  * example legacyReportingName: `test3(int)[1]`
* increases test count for each test class by 2 because in junit
  * class level exceptions are reported at class level
  * dependency exceptions are reported at junit level

### Sample Test Output
#### Before fix
```
<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<testcase name="com.facebook.buck.mytest.MyTest" runner_capabilities="simple_test_selector"                testprotocol="1.0">
  <test name="test" success="true" suite="com.facebook.buck.mytest.MyTest" time="17" type="SUCCESS"/>
  <test name="test2" success="true" suite="com.facebook.buck.mytest.MyTest" time="1" type="SUCCESS"/>
</testcase>
```

#### After fix
```
<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<testcase name="com.facebook.buck.mytest.MyTest" runner_capabilities="simple_test_selector" testprotocol="1.0">
  <test name="test" success="true" suite="com.facebook.buck.mytest.MyTest" time="11" type="SUCCESS"/>
  <test name="test2" success="true" suite="com.facebook.buck.mytest.MyTest" time="1" type="SUCCESS"/>
  <test name="com.facebook.buck.mytest.MyTest" success="true" suite="com.facebook.buck.mytest.MyTest" time="2" type="SUCCESS"/>
  <test name="JUnit Jupiter" success="true" suite="com.facebook.buck.mytest.MyTest" time="5" type="SUCCESS"/>
</testcase>
```

### Test Failure Example 1
BUCK file missing dependency `//third-party/java/opentest4j:opentest4j`

#### Before Fix
```
$ ./buck-out/gen/ce9b6f2e/programs/buck.pex test test/com/facebook/buck/mytest: -f MyTest#test
Building: finished in 2.3 sec (100%) 21/21 jobs, 5 updated
  Total time: 2.9 sec
Testing: finished in 1.0 sec
RESULTS FOR SELECTED TESTS
NO TESTS RAN
```

#### After Fix
```
$ ./buck-out/gen/ce9b6f2e/programs/buck.pex test test/com/facebook/buck/mytest: -f MyTest#test
FAILURE com.facebook.buck.mytest.MyTest JUnit Jupiter: TestEngine with ID 'junit-jupiter' failed to execute tests
Building: finished in 2.1 sec (100%) 21/21 jobs, 5 updated
  Total time: 2.7 sec
Testing: finished in 1.0 sec (0 PASS/1 FAIL)
RESULTS FOR SELECTED TESTS
FAIL    <100ms  0 Passed   0 Skipped   1 Failed   com.facebook.buck.mytest.MyTest
FAILURE com.facebook.buck.mytest.MyTest JUnit Jupiter: TestEngine with ID 'junit-jupiter' failed to execute tests
org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to execute tests
Caused by: java.lang.NoClassDefFoundError: org/opentest4j/TestAbortedException
Caused by: java.lang.ClassNotFoundException: org.opentest4j.TestAbortedException
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:476)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:594)
	at java.base/java.net.FactoryURLClassLoader.loadClass(URLClassLoader.java:904)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:527)

TESTS FAILED: 1 FAILURE
Failed target: //test/com/facebook/buck/mytest:mytest
FAIL com.facebook.buck.mytest.MyTest
```

### Test Failure Example 2
```
@testinstance(TestInstance.Lifecycle.PER_CLASS)
public class MyTest {
  @BeforeAll
  public void beforeAll() {
    throw new RuntimeException("before all exception");
  }

  @test
  public void test() {
  }
}
```

#### Before Fix
```
$ ./buck-out/gen/ce9b6f2e/programs/buck.pex test test/com/facebook/buck/mytest: -f MyTest#test
Building: finished in 0.4 sec (100%) 23/23 jobs, 5 updated
  Total time: 0.5 sec
Testing: finished in 0.6 sec
RESULTS FOR SELECTED TESTS
NO TESTS RAN
```

#### After Fix
```
$ ./buck-out/gen/ce9b6f2e/programs/buck.pex test test/com/facebook/buck/mytest: -f MyTest#test
FAILURE com.facebook.buck.mytest.MyTest MyTest: before all exception
Building: finished in 2.2 sec (100%) 23/23 jobs, 23 updated
  Total time: 2.8 sec
Testing: finished in 1.1 sec (1 PASS/1 FAIL)
RESULTS FOR SELECTED TESTS
FAIL    <100ms  1 Passed   0 Skipped   1 Failed   com.facebook.buck.mytest.MyTest
FAILURE com.facebook.buck.mytest.MyTest MyTest: before all exception
java.lang.RuntimeException: before all exception
	at com.facebook.buck.mytest.MyTest.beforeAll(MyTest.java:16)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)

TESTS FAILED: 1 FAILURE
Failed target: //test/com/facebook/buck/mytest:mytest
FAIL com.facebook.buck.mytest.MyTest
```
  • Loading branch information
ztai-add authored Aug 13, 2024
2 parents 21bb1ca + d1b0222 commit ec1a93b
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 8 deletions.
9 changes: 1 addition & 8 deletions src/com/facebook/buck/testrunner/Junit5TestListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ public void executionSkipped(TestIdentifier testIdentifier, String reason) {
// compare to Junit4TestListener.testStarted
@Override
public void executionStarted(TestIdentifier testIdentifier) {
if (!testIdentifier.isTest()) {
return;
}
// Create an intermediate stdout/stderr to capture any debugging statements (usually in the
// form of System.out.println) the developer is using to debug the test.
originalOut = System.out;
Expand Down Expand Up @@ -100,10 +97,6 @@ public void executionStarted(TestIdentifier testIdentifier) {
// compare to Junit4TestListener.testFinished
@Override
public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult testExecutionResult) {
if (!testIdentifier.isTest()) {
return;
}

// capture testIdentifier
this.testIdentifier = testIdentifier;

Expand Down Expand Up @@ -159,7 +152,7 @@ public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult
}

String className = testClass.getCanonicalName();
String methodName = testIdentifier.getDisplayName().replace("()", "");
String methodName = testIdentifier.getLegacyReportingName().replace("()", "");
long runTime = System.currentTimeMillis() - this.testStartTime;
results.add(
new TestResult(
Expand Down
7 changes: 7 additions & 0 deletions third-party/java/junit5-jupiter-engine/BUCK
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
prebuilt_jar(
name = "junit5-jupiter-engine",
binary_jar = "junit-jupiter-engine-5.10.1.jar",
javadoc_url = "https://junit.org/junit5/docs/5.10.1/api/",
source_jar = "junit-jupiter-engine-5.10.1-sources.jar",
visibility = ["PUBLIC"],
)
Binary file not shown.
Binary file not shown.
6 changes: 6 additions & 0 deletions third-party/java/opentest4j/BUCK
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
prebuilt_jar(
name = "opentest4j",
binary_jar = "opentest4j-1.3.0.jar",
source_jar = "opentest4j-1.3.0-sources.jar",
visibility = ["PUBLIC"],
)
Binary file not shown.
Binary file not shown.

0 comments on commit ec1a93b

Please sign in to comment.