eclipse/microprofile-open-api

OASFactoryErrorTest TCK test fails when it runs as the first test in the testsuite

TomasHofman opened this issue · 3 comments

This is observed in the 3.1 stream, I didn't test the 4.0 stream but I think the issue is present there too.

The OASFactoryErrorTest seems to not be self-sufficient, depends on initialization done by OASScanConfigTests.

This can result in test failures when the tests run in a wrong order. The test order is however random / undefined.

How the test fails

[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   OASFactoryErrorTest.baseInterfaceTest » Test 
Expected exception of type class...
[ERROR]   OASFactoryErrorTest.customAbstractClassTest » Test 
Expected exception of type...
[ERROR]   OASFactoryErrorTest.customClassTest » Test 
Expected exception of type class j...
[ERROR]   OASFactoryErrorTest.extendedBaseInterfaceTest » Test 
Expected exception of ty...
[ERROR]   OASFactoryErrorTest.extendedInterfaceTest » Test 
Expected exception of type c...
[ERROR]   OASFactoryErrorTest.nullValueTest » Test 
Expected exception of type class jav...
[INFO] 
[ERROR] Tests run: 32, Failures: 6, Errors: 0, Skipped: 0

The actual exceptions look like:

[ERROR] extendedInterfaceTest(org.eclipse.microprofile.openapi.tck.OASFactoryErrorTest)  Time elapsed: 0.023 s  <<< FAILURE!
org.testng.TestException: 

Expected exception of type class java.lang.IllegalArgumentException but got org.testng.TestException: 
Expected exception of type class java.lang.IllegalArgumentException but got java.lang.IllegalStateException: No OASFactoryResolver implementation found!
	at org.testng.internal.ExpectedExceptionsHolder.wrongException(ExpectedExceptionsHolder.java:68)
...
Caused by: java.lang.IllegalStateException: No OASFactoryResolver implementation found!
	at org.eclipse.microprofile.openapi.api@3.1.1//org.eclipse.microprofile.openapi.spi.OASFactoryResolver.instance(OASFactoryResolver.java:82)
	at org.eclipse.microprofile.openapi.api@3.1.1//org.eclipse.microprofile.openapi.OASFactory.createObject(OASFactory.java:96)
	at deployment.861d05f1-f0ea-4d5f-9f94-a6097513dfa3.war//org.eclipse.microprofile.openapi.tck.OASFactoryErrorTest.extendedInterfaceTest(OASFactoryErrorTest.java:103)

Reproduction

I can reproduce the problem on current Wildfly. Pre-build the project with following command:

mvn clean install  -DallTests -DskipTests -Dcheckstyle.skip

Then:

  • Running OASFactoryErrorTest alone fails: mvn test -pl :wildfly-ts-integ-mp-openapi -DallTests -Dtest=OASFactoryErrorTest
  • Running the full testsuite in the tck module succeeds (or fails if the OASFactoryErrorTest happens to run first): mvn test -pl :wildfly-ts-integ-mp-openapi -DallTests
  • Running just OASScanConfigTests,OASFactoryErrorTest succeds if the OASScanConfigTests runs first, fails if not.

Note: I'm enforcing the order in the third example by introducing a suite.xml file:

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
<suite name="Component Tests" verbose="2" annotations="JDK">
  <test name="test-subset" preserve-order="true" >
    <classes>
      <!-- Here list the test classes in desired order: -->
      <class name="org.eclipse.microprofile.openapi.tck.OASFactoryErrorTest" />
      <class name="org.eclipse.microprofile.openapi.tck.OASScanConfigTests" />
    </classes>
  </test>
</suite>

and then configuring the surefire plugin to use that file:

            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <configuration>
                    ...
                    <suiteXmlFiles>
                        <file>src/test/resources/suite.xml</file>
                    </suiteXmlFiles>
                </configuration>
            </plugin>

With that, the two tests in given order are run with mvn test -pl :wildfly-ts-integ-mp-openapi -DallTests.

What I think can be wrong with the test

The OpenAPI Spec states following in the section 5.1 (https://download.eclipse.org/microprofile/microprofile-open-api-3.1.1/microprofile-openapi-spec-3.1.1.html#_overview):

A fully processed OpenAPI document must be served from the root URL /openapi in response to an HTTP GET request if any of the following conditions are met:

* an OASModelReader has been configured with mp.openapi.model.reader

* an OASFilter has been configured with mp.openapi.filter

* one of the allowed static files is present, i.e. META-INF/openapi.(json|yaml|yml)

* the application uses JAX-RS

Note the last point - the app uses JAX-RS.

Checking the OASFactoryErrorTest, it uses an empty deployment, where where there are no REST annotations present:

    @Deployment
    public static WebArchive createDeployment() {
        return ShrinkWrap.create(WebArchive.class);
    }

I don't think any of the other conditions are configured either.

The reason why the wrong exception (IllegalStateException instead of IllegalArgumentException) is throw from OASFactory.createObject(...); is that the OASFactoryResolver has no instance initialized, so the OASFactoryResolver.instance() call throws IllegalStateException("No OASFactoryResolver implementation found!"), which is not supposed to happen.

I'm not quite sure if the OASFactoryResolver.instance() should get initialized outside of the conditions specified in the spec section about serving the "/openapi" endpoint:

  • an OASModelReader has been configured with mp.openapi.model.reader
  • an OASFilter has been configured with mp.openapi.filter
  • one of the allowed static files is present, i.e. META-INF/openapi.(json|yaml|yml)
  • the application uses JAX-RS

It could well be a problem in the Wildfly integration code, which is not doing something it should be doing.

A "fix" I was thinking about is to alter the OASFactoryErrorTest in a way that would satisfy those listed conditions, e.g. adding a REST class or a reader/filter configuration into the test deployment. Not sure if that's the right thing to do though.

I found this ticket that looks related: #413

Now it looks more like a problem in the TCK test to me. Why should the OASFactoryResolver.instance() be initialized when none of the four conditions mentioned above holds true?

#413 limits the cases where the endpoint is required to be active, but it doesn't say anything about whether OASFactoryResolver.instance() is allowed to not work.

That said, I think it's reasonable to change this test to be a more typical use-case where a reader is included.