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

Added support for running the TCK in Arquillian #84

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

jroper
Copy link
Member

@jroper jroper commented Aug 2, 2018

This provides a test class that can be run in Arquillian. I have tested this with Thorntail (using an Arquillian AuxillaryArchiveAppender to add in my own implementation), and it works.

I did originally try and make this work better with Arquillian, by trying to have it deploy each test to the container. I was able to work around the fact that the tests are created by a factory by writing my own custom container TestRunner that gets them from the factory. I was able to work around the fact that none of the tests extend the TestNG Arquillian class by extracting all its logic into a TestNG listener that was applied to the whole suite. And finally, I was able to work around the fact that none of the test classes has an @Deployment annotated method on it, by writing a custom DeploymentScenarioGenerator that extracted the deployment from a different class - the same class for each test. This was the most annoying one because WildFly has its own custom one, so doing this in a generic way was impossible. So, in the end it worked, however, it was a lot of what felt like very fragile code, so my strategy in the end was simply to write one Arquillian test that has one test method, and then, in that test method, instantiate a TestNG runner that runs the entire suite for me, using a CDI injected ReactiveStreamsEngine. If you run this from the IDE, it says there's only one test, but the logging shows 1100 or so tests, and makes it clear which failed and what the failures are.

Also fixed two bugs in the TCK where the default engine was loaded instead of the passed in one.

This provides a test class that can be run in Arquillian. I have tested
this with Thorntail (using an Arquillian AuxillaryArchiveAppender to add
in my own implementation), and it works.

I did originally try and make this work better with Arquillian, by
trying to have it deploy each test to the container. I was able to work
around the fact that the tests are created by a factory by writing my
own custom container test runner that gets them from the factory. I was
able to work around the fact that none of the tests extend the TestNG
Arquillian by extracting all its logic into a TestNG listener that was
applied to the whole suite. And finally, I was able to work around the
fact that none of the test classes has an @deployment annotated method
on it, by writing a custom DeploymentScenarioGenerator that extracted
the deployment from a different class - the same class for each test.
This was the most annoying one because WildFly has its own custom one,
so doing this in a generic way was impossible. So, in the end it
worked, however, it was a lot of what felt like very fragile code, so my
strategy in the end was simply to write one Arquillian test that has one
test method, and then, in that test method, instantiate a TestNG runner
that runs the entire suite for me, using a CDI injected
ReactiveStreamsEngine. If you run this from the IDE, it says there's
only one test, but the logging shows 1100 or so tests, and makes it
clear which failed and what the failures are.

Also fixed two bugs in the TCK where the default engine was loaded
instead of the passed in one.
@jroper jroper requested a review from cescoffier August 2, 2018 06:43
@jroper jroper added the streams label Aug 2, 2018
@cescoffier
Copy link
Contributor

@lordofthejars can you have a look to be sure it follows the recommendations?

@jroper the strategy looks good to me, but I don't understand how to run it. Adding a README would be very beneficial. I only used Arquillian with Junit so far.

@jroper
Copy link
Member Author

jroper commented Aug 2, 2018

It should be a matter of running the ReactiveStreamsArquillianTck. IntelliJ unfortunately doesn't let you run test classes from jars, so to work around that, create a class that extends it, and then run that.

@jroper
Copy link
Member Author

jroper commented Aug 3, 2018

@cescoffier Here's an example of using it. If you're testing against a container that has an implementation of mp rs ops built in, then you shouldn't need any of the Arquillian extension code, that's just to add an implementation to the container.

lightbend/microprofile-reactive-streams#8

@jroper
Copy link
Member Author

jroper commented Aug 3, 2018

Documentation added in #85.

@cescoffier
Copy link
Contributor

@jroper got it to work. I needed to add a producer creating the engine instance. The producer had to be in a specific package.

@jroper
Copy link
Member Author

jroper commented Aug 7, 2018

Merging as it was decided in the weekly hangout that this is the right approach.

@jroper jroper merged commit 6387275 into eclipse:master Aug 7, 2018
@jroper jroper deleted the streams-tck-with-arquillian branch August 7, 2018 19:05
@lordofthejars
Copy link

lordofthejars commented Aug 7, 2018 via email

@jroper
Copy link
Member Author

jroper commented Aug 7, 2018

@lordofthejars No worries.

Just so we're clear - there are two different TCKs in MicroProfile Reactive, one for the streams spec, that's this one, and one for the messaging spec, that's the one where we've done the most work with Arquillian and talked about in previous hangouts. The streams spec has no dependence on CDI or a container (and we hope to make it part of the JDK one day), it's just a stand alone library, hence, it doesn't need (and shouldn't require) Arquillian to run. So it provides about a hundred test classes that don't and can't have, for example, Arquillian deployment annotations on them.

That said, some vendors are going to want to run this TCK against their container. So, this PR introduces a new artifact that can run the existing TCK in a container using Aqruillian. See the original description on this PR for my attempts to do this in a more idiomatic Arquillian way defining extensions to dynamically add deployments to test classes that didn't use an Arquillian runner etc, I got it working but it felt too complex and hacky.

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

Successfully merging this pull request may close these issues.

3 participants