[ 
https://issues.apache.org/jira/browse/SUREFIRE-2144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17680109#comment-17680109
 ] 

Geoff Soutter commented on SUREFIRE-2144:
-----------------------------------------

Did a bit more static analysis and checked the JUnit 4 code from github.

There in RunnerBuilder, it does this

{code}
    public Runner safeRunnerForClass(Class<?> testClass) {
        try {
            Runner runner = runnerForClass(testClass);
            if (runner != null) {
                configureRunner(runner);
            }
            return runner;
        } catch (Throwable e) {
            return new ErrorReportingRunner(testClass, e);
        }
    }
{code}

ErrorReportingRunner is the thing that synthesizes the fake tests. 

{code}
    private Description describeCause() {
        return Description.createTestDescription(classNames, 
"initializationError");
    }
{code}

The issue is that SuiteMethodBuilder overrides RunnerBuilder, but doesn't 
override safeRunnerForClass, which provides behaviour which is not safe for 
Suites (i.e. generated fake fails in place of the actual generated tests).

So from my static analysis, seems this is a JUnit issue rather than Surefire 
issue.

> Using JUnit4 TestSuite to create test dynamically, synthetic 
> initializationError failure breaks Jenkins test history
> --------------------------------------------------------------------------------------------------------------------
>
>                 Key: SUREFIRE-2144
>                 URL: https://issues.apache.org/jira/browse/SUREFIRE-2144
>             Project: Maven Surefire
>          Issue Type: Bug
>    Affects Versions: 2.22.2
>            Reporter: Geoff Soutter
>            Priority: Major
>
> My team is dynamically creating tests using the JUnit3/4 static suite method 
> + TestSuite API
> {code}
>     public static junit.framework.Test suite() {
>         TestSuite testSuite = new TestSuite(xxx);
>         for (Test test: tests) {
>             testSuite.addTest(yyy);
>         }
>         return testSuite;
>     }
> {code}
> and then running this using Jenkins CI with Surefire.
> There is a nasty failure pattern which periodically deletes the Jenkins test 
> history - it resets the Age of all tests in Jenkins back to 1. The history / 
> Age report in Jenkins is key for us as it reveals which commit caused the 
> failure. We definitely do not want to lose that information.
> The failure pattern goes like this:
> * many tests are running fine, with some failures
> * a commit is made, CI triggers, Jenkins runs surefire.
> ** This results in a problem inside the suite() method, which throws a 
> RuntimeException. This is the dynamic test creation phase, before any tests 
> are run.
> ** This results in Surefire reporting a successful run of a single 
> "fake/synthetic" test which is reported as failed.
> * a commit is made to fix the test creation phase, CI again triggeers, 
> Jenkins runs surefire
> ** This results in many tests again running fine, with the same failures as 
> before
> ** However, Jenkins now reports all the old failures from the first step as 
> Age 1 - all the Age history is lost
> The synthetic test failure looks like so:
> {code}
>  [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 
> 1.064 s <<< FAILURE! - in com.company.package.MySuite
>  [ERROR] initializationError(com.company.package.MySuite) Time elapsed: 0.019 
> s <<< ERROR!
>  java.lang.RuntimeException: java.net.ConnectException: Connection refused 
> (Connection refused)
>  ...
>  at com.company.package.MySuite.suite(MySuite.java:xx)
>  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>  at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>  at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>  at java.lang.reflect.Method.invoke(Method.java:498)
>  at 
> org.junit.internal.runners.SuiteMethod.testFromSuiteMethod(SuiteMethod.java:35)
>  at org.junit.internal.runners.SuiteMethod.<init>(SuiteMethod.java:24)
>  at 
> org.junit.internal.builders.SuiteMethodBuilder.runnerForClass(SuiteMethodBuilder.java:11)
>  at 
> org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
>  at 
> org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
>  at 
> org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
>  at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:33)
>  at 
> org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:362)
>  at 
> org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
>  at 
> org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
>  at 
> org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
>  at 
> org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
>  at 
> org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
>  at 
> org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
>  at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
>  Caused by: java.net.ConnectException: Connection refused (Connection refused)
>  ...
>  ... 23 more
>  
>  [INFO]
>  [INFO] Results:
>  [INFO]
>  [ERROR] Errors:
>  [ERROR] MySuite.suite:xx ยป Runtime java.net.ConnectException: Connection 
> refused (...
>  [INFO]
>  [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
>  [INFO]
>  [INFO] 
> ------------------------------------------------------------------------
>  [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> {code}
> There is no "initializationError" test in our source code. I presume Surefire 
> has created it.
> It seems that the Jenkins JUnit report analysis gets completely confused when 
> it does the history analysis in this case. Presumably it tries to compare 
> * the "many" passing and failing tests from run N-1 with 
> * the single fake test from run N 
> After that I presume it decides that those many N-1 tests have been deleted, 
> and therefore deletes the history of those tests.
> So it seems that if we value the history analysis, we need to keep the test 
> names stable. If so, I suspect the fix is here is that Surefire should never 
> create and pretend to run synthetic tests. Rather, the entire run should 
> simply fail - I presume throw an exception out of the surefire plugin back to 
> maven? 
> Hopefully this would then prevent Jenkins from doing the test analysis and it 
> will not break the history / Age reporting.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to