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

[RESTEASY-2581] Add profile to enable testing using Wildfly "standalone-microprofile.xml" configuration file #2399

Merged
merged 1 commit into from May 22, 2020

Conversation

fabiobrz
Copy link

@fabiobrz fabiobrz commented May 10, 2020

Fixes https://issues.redhat.com/browse/RESTEASY-2581

Still WIP because some test cases still need for a little investigation.
Basically, this PR introduces a new profile which - like for what happens in Wildfly TS - would use the standalone-microprofile.xml config file to start the server up.
Then, when running tests by passing a new property - namely -Dts.standalone.microprofile to activate the above mentioned profile, some tests in integration-tests and integration-tests-spring/inmodule TS modules will fail.
Hence, two new JUint Category interfaces have been added:

  • ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11: skips tests that seem to fail because of the SmallRye MP OpenAPI implementation used in Wildfly 19 (further investigation to confirm this is needed)
  • ExpectedFailingWithStandaloneMicroprofileConfiguration: skips tests that fail because of missing subsystems in Wildfly when standalone-microprofile.xml is used.

This PR also bumps server.version to 19.0.0.Final.

Commands to verify that TS is running fine with the new profile:

$ export SERVER_VERSION=19.0.0.Final
$ mvn -B -am -pl integration-tests -fae -Dserver.version=$SERVER_VERSION install -Dts.standalone.microprofile

@fabiobrz fabiobrz changed the title WIP: Add profile to enable testing using Wildfly "standalone-microprofile.xml" configuration file WIP: [RESTEASY-2581] Add profile to enable testing using Wildfly "standalone-microprofile.xml" configuration file May 11, 2020
@fabiobrz fabiobrz force-pushed the add-mp-config-profiles branch 6 times, most recently from 2a9f6ef to 1974c31 Compare May 11, 2020 16:11
@rsearls
Copy link
Contributor

rsearls commented May 12, 2020

Shouldn't information about -Dts.standalone.microprofile be added to the README.MD?

@ronsigal ronsigal added the 3.11 label May 14, 2020
which are in standalone-full.xml but not in
standalone-microprofile.xml
-->
<additional.surefire.excluded.groups>org.jboss.resteasy.category.ExpectedFailingWithStandaloneMicroprofileConfiguration,org.jboss.resteasy.category.ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11</additional.surefire.excluded.groups>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 issue needs to be investigated. (cc @tommaso-borgato)

@marekkopecky
Copy link
Contributor

Shouldn't information about -Dts.standalone.microprofile be added to the README.MD?

+1 (cc @tommaso-borgato )

@fabiobrz fabiobrz changed the title WIP: [RESTEASY-2581] Add profile to enable testing using Wildfly "standalone-microprofile.xml" configuration file [RESTEASY-2581] Add profile to enable testing using Wildfly "standalone-microprofile.xml" configuration file May 14, 2020
@fabiobrz
Copy link
Author

Hi @rsearls and @marekkopecky, I added details to the README.md. Tests are passing with JDK11 and JDK8 locally, so I removed the WIP.

@marekkopecky
Copy link
Contributor

marekkopecky commented May 14, 2020

@fabiobrz I don't prefer to merge this now, because we don't know the reasons of OpenAPI errors (AFAIK). Thank you for readme updates.

@fabiobrz
Copy link
Author

fabiobrz commented May 14, 2020

I closed and reopened the PR in order to trigger Travis CI again as it failed despite the build succeeds both with JDK8 and JDK11 (OpenJDK) locally.

@ronsigal
Copy link
Collaborator

@fabiobrz, I merged the PR for branch 3.12 and then saw @marekkopecky's remark here. What do you think about the OpenAPI errors? If you think they need more investigation, could you create a JIRA for investigating

@fabiobrz
Copy link
Author

Hi @ronsigal, I see that a PR to produce a release to solve the issues with MP OpenAPI has been filed already: smallrye/smallrye-open-api#325 and I think the release will come soon and hope that the artifact version will be bumped in Wildfly soon, as well.
@marekkopecky WDYT?

@marekkopecky
Copy link
Contributor

marekkopecky commented May 15, 2020

If you think they need more investigation, could you create a JIRA for investigating
Investigation is done by @tommaso-borgato ( https://issues.redhat.com/browse/JBEAP-19495 ). In my PoV, this PR should contain links or additional description of ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 annotation. But if the issue will be fixed soon, we will have to remove this ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 annotation at all soon as well :)

Btw, Tommasso see test errors marked by ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 also on JDK8

@marekkopecky
Copy link
Contributor

In my PoV, this PR should contain links or additional description of ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 annotation

Thinking about this, this can be done in some following MR, so I'm ok with merging this MR now

@marekkopecky
Copy link
Contributor

marekkopecky commented May 15, 2020

@ronsigal (cc @tommaso-borgato ) Please let me know, if you prefer to add jira links to this PR directly or if you want any other changes in this PR in order to merge this PR

@ronsigal
Copy link
Collaborator

Hi @marekkopecky,

Thanks for looking at this. Actually, I merged #2407 into branch 3.12 yesterday. Branch 3.11 is dormant, except, maybe, for backporting something.

I want to release 3.12.0.Final today, to leave a little time before the feature freeze. If smallrye/smallrye-open-api#325 were merged today, that would be good, but, since we're talking about tests, I don't think it's that urgent. I also suggested a new JIRA for pursuing the failures for 3.13.0.Final.

-Ron

@fabiobrz
Copy link
Author

Hi @marekkopecky (CC @ronsigal and @tommaso-borgato), this is just to clarify that ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 is about failure due to SmallRye OpenAPI 1.1 implementation - as detailed in the Javadoc comment to the interface - rater than to the JDK(11).

@fabiobrz
Copy link
Author

Hi @ronsigal and CC @marekkopecky, RESTEasy JIRA was filed to track issues with MP OpenAPI, see https://issues.redhat.com/browse/RESTEASY-2589

@ronsigal
Copy link
Collaborator

I'll close this since it's superceded by #2407, now merged into branch 3.12.

@ronsigal ronsigal closed this May 16, 2020
@marekkopecky
Copy link
Contributor

Actually, I merged #2407 into branch 3.12 yesterday. Branch 3.11 is dormant, except, maybe, for backporting something.

@ronsigal actually, this backporting would be useful for QE, I added also some details to RESTEASY-2589 jira. Can you please consider to merge this PR?

@ronsigal
Copy link
Collaborator

Hi @marekkopecky,

Ok, sure, I don't think it should be a problem. I didn't know 3.11 was of any interest.

-Ron

@marekkopecky
Copy link
Contributor

marekkopecky commented May 21, 2020

@ronsigal Thanks, let me know if you want some additional actions from us (this PR is still not merged).

@ronsigal ronsigal reopened this May 22, 2020
@ronsigal
Copy link
Collaborator

Hi @marekkopecky,

Sorry, I forgot about this one. By the way, I see that smallrye/smallrye-open-api#325 has been closed. Are you able to upgrade smallrye-openapi?

-Ron

@ronsigal
Copy link
Collaborator

@marekkopecky, I'll wait for the tests to run and then merge.

@fabiobrz
Copy link
Author

fabiobrz commented May 22, 2020

Thanks @ronsigal! FYI and also CC @marekkopecky and @tommaso-borgato I noticed that some tests in the spring-inmodule module could be actually failing due to the SmallRye OpenAPI < 1.1.22 issue. I'll verify this later on but in such case an adjustment could be needed for this PR as well as a new one for the 3.12 branch. Of course - as all of the relevant tests here involve Wildfly integration - we must wait to have the SmallRye version bump in there or at least bump it by ourselves and see how it goes.
@pferraro maybe you know when the component version will be upgraded in Wildfly?
Hope this makes sense and let me know what you think.

@pferraro
Copy link

@fabiobrz WildFly uses version 1.2.3 of smallrye-open-api.

@ronsigal
Copy link
Collaborator

@marekkopecky, sorry for the delay.

@ronsigal ronsigal merged commit 5bd2ac9 into resteasy:3.11 May 22, 2020
@fabiobrz
Copy link
Author

Thanks @pferraro! I actually didn’t check the master branch. I see that these tests use Wildfly 19. My concern was abut this. I’ll take a closer look :)

@ronsigal
Copy link
Collaborator

Hey @marekkopecky, @fabiobrz, @tommaso-borgato,

It looks like the changes in smallrye/smallrye-open-api#325 have been applied to smallrye-open-api 1.2.3.

RESTEasy 3.11.2.Final doesn't have any dependency on smallrye-open-api, direct or indirect, so I'm guessing all that RESTEasy would need would be removing the ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 tags, right? Would you guys need a 3.11.3.FInal release

@fabiobrz
Copy link
Author

Hi @ronsigal, the tests we’re talking about are supposed to verify integration with Wildfly (those in the spring-inmodule and spring-deployment as well). If I don’t go wrong (I can’t verify right now) 3.12 and 3.11 are using Wildfly 19.0.0.Beta/Final and those are producing the failures for which the annotation is needed at the moment. If the Wildfly version those branch would use is going to change to one which supports the new SmallRye OpenAPI release, then yes, I’d say the “ ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11“ annotation could be removed, IMO.

@ronsigal
Copy link
Collaborator

Hey @fabiobrz, we have a default WF version in the code, and then we have the versions we test with in travis / github actions. Right now, 3.x versions are testing with WF 18 and 19, and we should add 20 to that.

But I was under the impression that the ts.standalone.microprofile profile would be activated in WildFly or EAP testing. If that's true, then it would be enough to remove @ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 from any RESTEasy version that's targeting a version of WF / EAP that's got the upgraded smallrye-open-api, right? So for example, I suppose 3.13.0.Final will go into WF 21, so the annotation should be removed there, and there's no rush on that. But what about this PR into RESTEasy 3.11.2.Final? My understanding is that it will go into EAP 7.3.1 and EAP XP 1.0.0. Will they have the upgraded smallrye-open-api? If so, then you might want a RESTEasy 3.11.3.Final, right?

@fabiobrz
Copy link
Author

fabiobrz commented May 22, 2020

Hi @ronsigal, I think you summarised the whole thing perfectly. I am on the same page but I’d like to hear from @marekkopecky as well, mostly on what’s going on with EAP.
Cheers!

@fabiobrz
Copy link
Author

Thanks @ronsigal! FYI and also CC @marekkopecky and @tommaso-borgato I noticed that some tests in the spring-inmodule module could be actually failing due to the SmallRye OpenAPI < 1.1.22 issue. I'll verify this later on but in such case an adjustment could be needed for this PR as well as a new one for the 3.12 branch.
...

Hey @ronsigal, meantime I've checked this today and it seems to me that SmallRye OpenAPI implementation is unrelated with respect to tests in integration-tests/spring/inmodule so I've left ExpectedFailingWithStandaloneMicroprofileConfiguration on those to be skipped. Basically no need for another PR, neither here nor for 3.12.

@fabiobrz
Copy link
Author

fabiobrz commented May 25, 2020

Hi again @ronsigal.

But I was under the impression that the ts.standalone.microprofile profile would be activated in WildFly or EAP testing. If that's true, then it would be enough to remove @ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 from any RESTEasy version that's targeting a version of WF / EAP that's got the upgraded smallrye-open-api, right?

Yes, I do agree.

So for example, I suppose 3.13.0.Final will go into WF 21, so the annotation should be removed there, and there's no rush on that.

Yep, agree on this too and - as Paul stated earlier - it seems that the annotation could be removed from 3.12 too, which seems to be used by Wildfly 20 (https://github.com/wildfly/wildfly/blob/master/pom.xml#L391) which in turn is equipped with SmallRye OpenAPI 1.2.3, https://github.com/wildfly/wildfly/blob/master/pom.xml#L305 (fixed, I guess).

But what about this PR into RESTEasy 3.11.2.Final? My understanding is that it will go into EAP 7.3.1 and EAP XP 1.0.0. Will they have the upgraded smallrye-open-api?

Those have the upgraded SmallRye OpenAPI indeed. I've been checking the POM on which both the above mentioned distributions are based, and it uses RESTEasy 3.11.2 and SmallRye OpeAPI 1.1.22 (fixed).

If so, then you might want a RESTEasy 3.11.3.Final, right?

Well, yes. My understanding is that if you release a 3.11.3.Final withouth the ExpectedFailingBecauseOfSmallRyeMicroprofileOpenApi11 annotations, then next releases of 7.3.x and XP streams could bump the RESTEasy version to that one.

So, basically, IIUC, we need to file 2 PRs to remove the annotation both from 3.11 and 3.12 as it seems no longer needed now. I can file them. WDYT? CC @marekkopecky, @tommaso-borgato

@marekkopecky
Copy link
Contributor

we need to file 2 PRs to remove the annotation both from 3.11 and 3.12 as it seems no longer needed now

+1

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