-
Notifications
You must be signed in to change notification settings - Fork 19
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
Migrate all the extension examples to the testsuite #68
Conversation
That's awesome! Thanks! 🍻
Oh, looks like I missed this dependency. Feel free to remove it in this case. |
Signed-off-by: Gregor Tudan <gregor@tudan.de>
…r runtime dependencies like jython)
Okay, the test-suite passes on Payara. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very good : )
@gtudan Do you think we should merge this one now and address the Wildfly issues later or whould we first try to fix the Wildfly issues as part of this pull request? |
Of course I would prefer the latter. But if this isn't easy, we could also merge the PR first and address the Wildfly issues later. |
Hi Christian! I'm afraid fixing the failing tests on wildly will take a while, as there are some nasty hacks required. I.e. the Asciidoctor-Example requires a custom module to be preinstalled, which quiet hard to do with docker and arquillian. https://github.com/asciidoctor/asciidoctorj/blob/26641dbe1510a83fbb9c5cab3974a8736b385bdd/README.adoc#running-asciidoctorj-on-wildfly-as I'd prefer to merge this now to make sure we don't break anything before the release and open an issue for the failing tests on WF. |
The issue for the failing tests is #70 |
@gtudan Thanks a lot! In this case I'm fine with merging soon. But one issue came to my mind. I guess you added a few new dependencies for the new test suite, correct (Arquillian adapters or something similar)? In this case we have to file a few CQs before integrating the 3rd party dependencies. I can help with this. I just need some time to have a closer look at the pom diffs. |
Actually no, we layed the foundations for the test suite before we moved to eclipse, so everything was in place when the CQs were filed. This change does not add any dependencies that were not already in the example projects. But of course I‘ll double check the CQs to see if everything has been approved. I‘d highly appreciate your help if we find anything missing!
… Am 13.05.2019 um 20:45 schrieb Florian Hirsch ***@***.***>:
@lefloh approved this pull request.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Looks like we’re lucky, all of the dependencies for the testsuite were in the test CQ:
https://dev.eclipse.org/ipzilla/show_bug.cgi?id=18898
… Am 13.05.2019 um 23:04 schrieb Gregor Tudan ***@***.***>:
Actually no, we layed the foundations for the test suite before we moved to eclipse, so everything was in place when the CQs were filed. This change does not add any dependencies that were not already in the example projects. But of course I‘ll double check the CQs to see if everything has been approved. I‘d highly appreciate your help if we find anything missing!
> Am 13.05.2019 um 20:45 schrieb Florian Hirsch ***@***.***>:
>
> @lefloh approved this pull request.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
|
@gtudan Awesome! Thanks for checking. In this case I'm fine with merging now. Thanks a lot for working on this. |
This migrates all the examples for extensions to the testsuite from #28
There are still some issues on containers that I would like to fix before merging this. At least the wildfly issues look like quick fixes (mostly classloader issues).
I found a dependency to groovy-all in the jsr223-example. Do I remember right that this had an incompatible license? I removed it for now, as it's only a small test-case.