Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions thrown by extensions invisible in Maven build logs #1444

Closed
kriegaex opened this issue Mar 13, 2022 · 8 comments · Fixed by #1447
Closed

Exceptions thrown by extensions invisible in Maven build logs #1444

kriegaex opened this issue Mar 13, 2022 · 8 comments · Fixed by #1447
Labels

Comments

@kriegaex
Copy link
Contributor

kriegaex commented Mar 13, 2022

Describe the bug

As mentioned first in #1437 (comment) and then in #1442 (comment), if an annotation-driven extension throws a runtime exception, e.g. InvalidSpecException or ExtensionException, this is not reflected in the Maven build log, not even in debug mode. So I guess that either the JUnit 5 platform (my guess) or some part in Spock is swallowing those exceptions.

When running the same spec in an IDE like IntelliJ IDEA, the extension exception is shown. In Maven and probably also in Gradle, the whole affected spec is simply not added to the test plan, i.e. it is not reported as ignored or failed at all. Update: In Gradle, the spec is reported as failed.

This could affect a large number of built-in Spock extensions throwing exceptions, see the attached search log from my IDE.

To Reproduce

Add Spock annotations provoking errors to a test spec, e.g. a combination of @Unroll and @Rollup on the same spec or feature method. Run it, first from an IDE, then from Maven. (Continued in "actual behaviour" section.)

Expected behavior

  • The exception thrown by the extension is reported in the build log.
  • The spec or feature is question is reported as a test error (not a failure, because no test was executed).
  • The remaining specs or features are executed normally.

Actual behavior

Run the spec from IntelliJ IDEA. You should see something like

org.spockframework.runtime.InvalidSpecException: @Unroll and @Rollup must not be used on the same feature: run all tests with skip after first failure

  at org.spockframework.runtime.extension.builtin.UnrollExtension.visitFeature(UnrollExtension.java:74)
  at org.spockframework.runtime.extension.builtin.UnrollExtension.lambda$visitSpec$0(UnrollExtension.java:59)

Now run the same test from Maven, using a command like mvn test -DfailIfNoTests=false -Dtest=StepwiseIterationsTest. You should see something like:

[INFO] --- maven-surefire-plugin:2.22.2:test (default-test) @ perform-tests ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

Java version

JDK 17, but I do not think it matters.

Buildtool version

The problem occurs in Maven, not in Gradle.

  • Maven 3.8.1
  • Surefire 2.22.2 or latest Surefire 3.0.0-M6-SNAPSHOT, same behaviour
  • It is independent of whether the compilation is done by GMavenPlus Plugin or by Maven Compiler Plugin with Groovy-Eclipse. In my own projects I always use the latter, while the official Spock example project uses the former. The effect is the same.

What operating system are you using

Windows

Dependencies

x

Additional context

x

@kriegaex kriegaex added the bug label Mar 13, 2022
@kriegaex
Copy link
Contributor Author

kriegaex commented Mar 13, 2022

OK, I know more now: It seems to be Maven-specific, because in Gradle the test is reported as failed, while in Maven it is silently ignored. I tried with https://github.com/spockframework/spock-example. Simply clone it and add these two annotations to DataDrivenSpec:

@Unroll
@Rollup
class DataDrivenSpec extends Specification {

Then run ./gradlew clean test vs mvn clean test from the console and see the difference.

Gradle:

> Task :test
DataDrivenSpec > initializationError FAILED
    org.spockframework.runtime.InvalidSpecException at UnrollExtension.java:39
(...)
33 tests completed, 1 failed

Maven:

[INFO] Tests run: 34, Failures: 0, Errors: 0, Skipped: 0

Maven without the @Unroll @Rollup modification would report:

[INFO] Tests run: 45, Failures: 0, Errors: 0, Skipped: 0

@kriegaex
Copy link
Contributor Author

kriegaex commented Mar 13, 2022

@Tibor17 for Surefire and @marcphilipp for JUnit, can you maybe say something about why this might happen? This is a potentially big problem for Spock, failures not being reported in Surefire. It is super easy to reproduce, see the previous comment.

@kriegaex
Copy link
Contributor Author

@leonard84, I think that there might be something wrong with ErrorSpecNode. In

try {
runContext.createExtensionRunner(specNode.getNodeInfo()).run();
} catch (Exception e) {
return new ErrorSpecNode(specNode.getUniqueId(), configuration, specNode.getNodeInfo(),
e);
}
exceptions thrown by extensions are being handled - obviously in a way which is good enough for Gradle, but not for Maven Surefire. Maybe the way in which ErrorSpecNode is trying to avoid the spec node removal does not quite work for Maven. I have never looked into anything like this before, but if you are not faster than I, I am trying to dig in a little deeper.

@kriegaex
Copy link
Contributor Author

kriegaex commented Mar 13, 2022

If we look at org.apache.maven.surefire.junitplatform.TestPlanScannerFilter#accept, we see:

    @Override
    @SuppressWarnings( "rawtypes" )
    public boolean accept( Class testClass )
    {
        LauncherDiscoveryRequest discoveryRequest = request()
                        .selectors( selectClass( testClass.getName() ) )
                        .filters( includeAndExcludeFilters ).build();
        TestPlan testPlan = launcher.discover( discoveryRequest );
        return testPlan.containsTests();
    }

Somehow, for an ErrorSpecNode the condition testPlan.containsTests() is false, which leads to the error spect node to get filtered out from the test plan completely. I experimentally added

  @Override
  public boolean isTest() {
    return true;
  }

to ErrorSpecNode. When now I run the example project against a Spock 2.2 snapshot, I see

[INFO] Running DataDrivenSpec
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.002 s <<< FAILURE! - in DataDrivenSpec
[ERROR] DataDrivenSpec  Time elapsed: 0.002 s  <<< ERROR!
org.spockframework.runtime.InvalidSpecException: @Unroll and @Rollup must not be used on the same spec: DataDrivenSpec
(...)
[ERROR] Errors:
[ERROR]   DataDrivenSpec » InvalidSpec @Unroll and @Rollup must not be used on the same ...
[INFO]
[ERROR] Tests run: 35, Failures: 0, Errors: 1, Skipped: 0

That looks better, and in Gradle the output stays the same as before, so superficially nothing seems to be broken. But I am not sure if this is the right change. It was just a hack, and I do not know enough about the super-classes and -interfaces implemented by ErrorSpecNode to actually judge if this is a correct and sufficient change. I do hope that one of the three people I notified by mentioning their names before can provide additional insights here.

@leonard84
Copy link
Member

Out of curiosity, does this only happen if you use -Dtest=StepwiseIterationsTest or also without any filtering?

@kriegaex
Copy link
Contributor Author

kriegaex commented Mar 13, 2022

As you can see in the protocols in my comments when using the Spock example project, there I was running the full build in all cases without any filtering. So the answer is: Yes, it also happens without any filtering. Please note the number of tests listed in the console logs and the Maven and Gradle commands I used to build the project.

P.S.: Do you remember that a while ago when we talked about another Maven build problem with Spock which did not occur in Gradle, that I said it would be important to also have Maven integration tests? This is the second time within a short time frame that we have such cases. I am still thinking that the Spock CI build has a blind spot there.

kriegaex added a commit to kriegaex/spock that referenced this issue Mar 14, 2022
leonard84 pushed a commit that referenced this issue Mar 14, 2022
Prior to this commit, SpockEngineDiscoveryPostProcessor#processSpecNode creates an ErrorSpecNode (which implements org.junit.platform.engine.TestDescriptor) after catching errors thrown by extensions, which is then silently discarded by Maven Surefire because it filters for testPlan.containsTests(). Therefore, we have to make sure that containsTests returns true. An easy way to do that is to override mayRegisterTests(). Another way would be to override isTest() instead, but actually neither an error node nor a spec actually is as test, they rather can contain tests.

Fixes #1444
@Tibor17
Copy link

Tibor17 commented Mar 14, 2022

Hi @kriegaex
We usually work with an isolated test which helps us to reproduce a problem. Can you pls point me to such a test?

@kriegaex
Copy link
Contributor Author

kriegaex commented Mar 14, 2022

Hi @Tibor17. If you clone the tiny example project and modify one test the way I described, you get a full test run in 10 seconds.

Thank you so much for answering my call to look into this, but I think I fixed it already in the merged PR. Surefire is more picky than Gradle with accepting Spock's error nodes, but the tweak I had to make was pretty easy. The harder part was to find where the exception was swallowed. So in the end it was not a Surefire problem as such but rather Spock not catering to Surefire's specific needs. Nobody noticed before, because Spock is built and tested on Gradle only. I however am using Maven, so I noticed while trying to test and debug a PR for Spock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants