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

Jaxws bootable jar test #83

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

rsearls
Copy link
Collaborator

@rsearls rsearls commented Sep 5, 2023

Description

A jaxws test example
(fix #82)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • Pull Request contains a description of the changes
  • Pull Request does not include fixes for multiple issues/topics
  • Code is self-descriptive and/or documented
  • I have implemented unit tests to cover my changes
  • I tested my code in OpenShift

@openshift-ci
Copy link

openshift-ci bot commented Sep 5, 2023

Hi @rsearls. Thanks for your PR.

I'm waiting for a Intersmash member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@fabiobrz
Copy link
Member

fabiobrz commented Sep 5, 2023

/ok-to-test

@fabiobrz fabiobrz self-requested a review September 5, 2023 22:06
Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @rsearls! It's great that you started the demos module!

Since we want to bring in the examples incrementally, I'd start with just the jaxws test, and remove the following:

  • all the changes to the testsuite module
  • all the changes to the tools module
  • the whole demos/legacy-wildfly-shrinkwrap-examplemodule
  • the ws-bootable-jar-example/jaxrs module
  • the demos/ws-bootable-jar-example/ws-bootable-jar-example.iml file

BTW the testsuite/pom.xml conflict will disappear once the first bullet above is done.
I'll review the other parts meanwhile.

@rsearls rsearls force-pushed the jaxws-bootable-jar-test branch from 7cc2104 to 1a66edc Compare September 6, 2023 17:50
@fabiobrz
Copy link
Member

fabiobrz commented Sep 6, 2023

Hi @rsearls - I see you moved the test app deployment under the deployments module.
I know we talked about this, but I'd like to share some thoughts regarding this: if this is a PoC - as it seems to me by looking at the very basic test - then there's no need to move the application code to a new deployment into the deployments module
On the other side, I have some ideas on where to place actual Intersmash tests, and I'd like to discuss this with you - provided you don't have a place where to place your team tests already.

So please, keep the demos module, and please follow my hints here.

Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

Hi @rsearls - I had another round of review and I think we're moving in the right direction. I dropped some comments.

demos/pom.xml Outdated Show resolved Hide resolved
demos/ws-bootable-jar-example/pom.xml Outdated Show resolved Hide resolved
demos/ws-bootable-jar-example/pom.xml Outdated Show resolved Hide resolved
testsuite/pom.xml Outdated Show resolved Hide resolved
demos/ws-bootable-jar-example/jaxws/pom.xml Outdated Show resolved Hide resolved
@rsearls rsearls force-pushed the jaxws-bootable-jar-test branch 5 times, most recently from 6e226ff to 3bd23e8 Compare September 27, 2023 19:18
@fabiobrz
Copy link
Member

/test intersmash-e2e

Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

Hi @rsearls and thanks for addressing my comments.

It seems we're on the right way and almost done! I've replied to a couple of comments and suggested some changes, which BTW I've experimented a bit with, and that you can preview here.

Also, feel free to squash your commits so that we've got less changes when merging.

@fabiobrz
Copy link
Member

Hi @rsearls - I've noticed that wildfly bits were not bumped, so I added one commit to my topic branch for doing that, see fabiobrz@02725cd

@rsearls
Copy link
Collaborator Author

rsearls commented Sep 28, 2023

@fabiobrz
I've reviewed the CI msgs for this failure. The test is failing with a
cz.xtf.core.waiting.WaiterException

[2023-09-27 19:59:22,032] INFO - Waiting up to 1 minute. Reason: Cleaning project - intersmash-com
[2023-09-27 20:00:23,224] INFO - Cleaning namespace: intersmash-com - timed out.
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 62.611 s <<< FAILURE! - in org.jboss.intersmash.testsuite.provision.openshift.WildflyBootableJarTestCase
[ERROR] org.jboss.intersmash.testsuite.provision.openshift.WildflyBootableJarTestCase Time elapsed: 62.611 s <<< ERROR!
cz.xtf.core.waiting.WaiterException: Cleaning project - intersmash-com
at cz.xtf.core.waiting.SimpleWaiter.waitFor(SimpleWaiter.java:166)
at cz.xtf.junit5.extensions.CleanBeforeAllCallback.beforeAll(CleanBeforeAllCallback.java:12)

I've reviewed the code in WildflyBootableJarTestCase and its dependencies.
I have make no changes to this or related classes.
I have not found this test referencing any code related to demos.
In testsuite/pom.xml there were 2 dependency ref additions, jakarta.servlet-api
and jakarta.inject-api. Test WildflyBootableJarTestCase is not referencing code
in either of these archives.

Perhaps the wait time for cleanup should be extended.

@fabiobrz
Copy link
Member

/test prod-intersmash-e2e-prod

@fabiobrz
Copy link
Member

All Green now @rsearls, great job!
Could you please squash your commits, so we can move on and merge?

@rsearls rsearls force-pushed the jaxws-bootable-jar-test branch from 6946e18 to baf88e9 Compare September 29, 2023 00:55
@rsearls
Copy link
Collaborator Author

rsearls commented Sep 29, 2023

Squash complete

@rsearls rsearls force-pushed the jaxws-bootable-jar-test branch 3 times, most recently from 80d3fc2 to 03b5dd2 Compare September 29, 2023 13:46
@rsearls rsearls force-pushed the jaxws-bootable-jar-test branch from 03b5dd2 to a3fd7dc Compare September 29, 2023 13:59
@fabiobrz
Copy link
Member

/test all

@fabiobrz fabiobrz self-assigned this Sep 29, 2023
@fabiobrz
Copy link
Member

/test all

Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

Thanks @rsearls for contrbuting the first Intersmash demo! 😃
Changes LGTM, and I'll merge them as soon as you fill in the boxes in the description!

@rsearls
Copy link
Collaborator Author

rsearls commented Sep 29, 2023

done

@fabiobrz fabiobrz merged commit 958e020 into Intersmash:main Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a working jaxws test example
2 participants