jakartaee / platform-tck

Jakartaee-tck
Other
124 stars 103 forks source link

Standalone Persistence TCK runner inside Glassfish #1319

Open OndroMih opened 1 month ago

OndroMih commented 1 month ago

An improvement over https://github.com/jakartaee/platform-tck/pull/1317 to run the TCK tests deployed to a running GlassFish server.

All standalone tests pass except createEntityManagerFactoryNoBeanValidatorTest in se.entityManagerFactory.Client2, which doesn't make sense in a Jakarta EE server because Validation provider is always present.

Note that this still runs the tests in standalone mode. GlassFish should also run the tests in JakartaEE mode. This is also supported by this pull requests, it's enough to change the system property platform.mode in pom.xml from "standalone" to "jakartaEE". This will enable the JakartaEeExecutionInterceptor, which will attempt to inject resources into the test. However, I don't know how these resources should be created - in JakartaEE mode, there are no persistence.xml files with JTA persistence units to initialize the resources.

I will need some help to run the tests in "jakartaEE" mode, using all the supported test vehicles (https://github.com/jakartaee/platform-tck/tree/tckrefactor/common/src/main/java/com/sun/ts/tests/common/vehicle - only stateless3, stateful3, appmanaged, appmanagedNoTx, pmservlet, and puservlet are necessary for Persistence TCK). The built-in Arquillian protocol invokes the tests using a servlet. Different test vehicles might need to be implemented as different Arquillian protocols. Or with a single protocol which executes all tests using various vehicles.

scottmarlow commented 4 weeks ago

I will need some help to run the tests in "jakartaEE" mode, using all the supported test vehicles (https://github.com/jakartaee/platform-tck/tree/tckrefactor/common/src/main/java/com/sun/ts/tests/common/vehicle - only stateless3, stateful3, appmanaged, appmanagedNoTx, pmservlet, and puservlet are necessary for Persistence TCK). The built-in Arquillian protocol invokes the tests using a servlet. Different test vehicles might need to be implemented as different Arquillian protocols. Or with a single protocol which executes all tests using various vehicles.

JakartaEE mode is not yet ready for testing. https://github.com/jakartaee/platform-tck/wiki/OpenRewrite has some notes on the work in progress to add automation to for converting the EE tests.

https://gist.github.com/scottmarlow/e06a0d0e3269464784926df90518785c shows the current output of the Open Rewrite conversion (using the EE 10 Platform TCK zip as a model of what to include in each deployment).

We also need to take care to not update any Persistence Component TCK Client.java source that is already balloted for EE 11 so we don't have to repeat the Persistence 3.2 ballot (via a new release).

We also need to figure out where we want to keep the EE 10 Platform TCK test sources due to ^ concern.

We just added some initial code to read the test vehicles from the EE 10 Platform TCK src/vehicle.properties but a bit more is needed for that. We also need to generate code to deal with each vehicle from each test where there likely will be some common code for each vehicle.

arjantijms commented 4 weeks ago

Error: Failed to execute goal org.apache.maven.plugins:maven-failsafe-plugin:3.2.5:verify (persistence-tests-1) on project persistence-tck-run-ee: Error:
Error: Please refer to /home/runner/work/platform-tck/platform-tck/glassfish-runner/persistence-tck/persistence-tck-run-ee/target/failsafe-reports for the individual test results. Error: Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream. Error: [ERROR] There was an error in the forked process Error: Java heap space Error: java.lang.OutOfMemoryError: Java heap space Error:
Error: at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:628)

@OndroMih @scottmarlow Something is out of memory, but I'm not exactly sure what that something is and how to give it more memory. Any ideas?

arjantijms commented 4 weeks ago

Running it locally, and it looks like the OOM happens on the junit/client side:

[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.57 s -- in ee.jakarta.tck.persistence.core.entitytest.detach.basic.Client
Jun 03, 2024 5:40:37 PM org.junit.platform.launcher.listeners.LoggingListener lambda$forJavaUtilLogging$0
INFO: Execution Finished: Client - [engine:junit-jupiter]/[class:ee.jakarta.tck.persistence.core.entitytest.detach.basic.Client] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
Jun 03, 2024 5:40:37 PM org.junit.platform.launcher.listeners.LoggingListener lambda$forJavaUtilLogging$0

INFO: Execution Started: Client - [engine:junit-jupiter]/[class:ee.jakarta.tck.persistence.core.entitytest.detach.manyXmany.Client]
[INFO] Running ee.jakarta.tck.persistence.core.entitytest.detach.manyXmany.Client
Jun 03, 2024 5:41:10 PM org.omnifaces.arquillian.container.glassfish.clientutils.GlassFishClientUtil getResponseMap
WARNING: While Deploying Application: package --exit_code: FAILURE, message: Resource not found. [status: CLIENT_ERROR reason: Not Found]
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 33.02 s <<< FAILURE! -- in ee.jakarta.tck.persistence.core.entitytest.detach.manyXmany.Client

[ERROR] ee.jakarta.tck.persistence.core.entitytest.detach.manyXmany.Client -- Time elapsed: 33.02 s <<< ERROR!
org.omnifaces.arquillian.ws.rs.ProcessingException: Java heap space
    at org.omnifaces.arquillian.jersey.client.ClientRuntime.invoke(ClientRuntime.java:309)
    at org.omnifaces.arquillian.jersey.client.JerseyInvocation.lambda$invoke$0(JerseyInvocation.java:662)
    at org.omnifaces.arquillian.jersey.client.JerseyInvocation.call(JerseyInvocation.java:697)
    at org.omnifaces.arquillian.jersey.client.JerseyInvocation.lambda$runInScope$3(JerseyInvocation.java:691)
OndroMih commented 4 weeks ago

Hi @scottmarlow,

We also need to take care to not update any Persistence Component TCK Client.java source

I think that this is exactly what the current OpenRewrite conversion does in this gist: https://gist.github.com/scottmarlow/e06a0d0e3269464784926df90518785c - it modifies the Client.java source. In this PR, I used a combination of Arquillian and JUnit extensions to deploy the test in a Jakarta EE container without modifying anything in the test sources. I think it's a better approach. The test stays agnostic to how it's executed, and different Jakarta EE implementations can modify the Arquillian deployment as they want, even use something else than Arquillian.

OndroMih commented 4 weeks ago

We had a great conversation with @scottmarlow on Slack today, I copy it here:

Ondro: In my PR (this PR) I show how all this can be dynamically added to a plain Java SE test using Arquillian and JUnit 5 extensions. Extensions can be enabled globally for all tests, and they can wrap the test and execute it in a Jakarta EE container using Arquillian, they can define the deployment without having @Deployment method in the test code, they can pass system properties to the server through a properties file packaged into the deployment, etc.

I think that vehicles can be implemented as Arquillian protocols, so that Arquillian invokes the application deployed to the server in different ways, e.g. once through a servlet that injects @PersistenceContext, another time using a servlet that injects @PersistenceUnit, and another time even through a remote EJB.

Scott Marlow: If you're going to generate code for vehicles into the test code, you'll need to maintain 2 sets of tests - one for Java SE, without Arquillian deployments, and another one with Arquillian deployments. I was thinking that the generated code would call the (extended) SE style code. But really do appreciate the alternative idea that we should consider!

Ondro: We already used this approach in Batch TCK for EE 10, we had a good discussion on Batch ML back then.

Scott Marlow: That worked out well, I should look at that again

Ondro: I based my PR (this PR) on the runner we created as an example runner in the Batch TCK.

The best thing about it is that the whole solution is just an example. Any implementation can modify it or replace with their own solution. And there's no need to maintain anything in the TCK itself, just plain Java SE tests.

Scott Marlow: I guess one question will be how much custom code each EE implementation has to come up with for the different vehicles,

For the open rewrite generated code approach, there would be less impact on the EE impls but that isn't entirely clear yet

Ondro: The runner I created for GlassFish can be used with any servlet-based Jakarta EE impl (Web profile or platform). There;s nothing specific to GlassFish.

In Batch TCK, we provide an example project that I think uses GlassFish as an example. It's just a matter of changing Arquillian configuration to deploy to another container.

So, the runner is not part of the TCK itself, it's not required to use it, but we still provide an example so that implementors can take it and modify to their liking. In most cases, it's just configuring Arquillian to use a different container.

Scott Marlow: Open ReWrite does seem fine for the shrinkwrap generation. Perhaps we can use ^ for the vehicle integration instead.

Ondro: For vehicles, it would be the same. As vehicles should rely on standard EE functionality (servlet, remote EJB, JSP invoked through HTTP), it's again only a matter of adapting the Arquillian configuration

Scott Marlow: The test vehicle handling isn't handled as well by the open rewrite approach so good timing that you are pushing this now!

Ondro: ShrinkWrap generation might be necessary if you want to verify certain ways of packaging things. For example, you want to verify that persistence.xml can be in a JAR inside a WAR. Then the test should contain code that generates this structure.

Scott Marlow: we have ear deployments also, a lot of those.

Ondro: In batch we didn't need this, the EE tests were identical to the SE tests. So we just got rid of the EE tests and described how to run plain SE tests in a container.

Scott Marlow: We aren't ready to drop EAR deployments as that is part of the Platform spec still.

https://gist.githubusercontent.com/scottmarlow/e06a0d0e3269464784926df90518785c/raw/60d5cc20d3f948eedfea9061c9ea6b36eb1c2878/gistfile1.txt contains a sample of the generated shrinkwrap code

arjantijms commented 4 weeks ago

@OndroMih What about merging this work anyway? Maybe changing the folder / artefact to "-se-in-ee" or something like that? This is still very valuable as a kind of smoke test to assert GF can run the basic Persistence tests. We do need to exclude the bean validation test though.

scottmarlow commented 4 weeks ago

Thanks for pasting the slack discussion here @OndroMih as it will soon be deleted as most of our our slack discussions have been deleted. I'm not yet sure of how this approach would work for ear deployments exactly but maybe.

With the EE 10 Platform TCK, for each specific JPA SE TCK test (and some EE only tests), we run that test multiple times against the different test vehicles mentioned in https://github.com/jakartaee/platform-tck/blob/10.0.x/install/jakartaee/other/vehicle.properties#L105, each success/failure is counted separately.

For EE 11, we (on Platform TCK team) discussed that we don't have enough time in the EE 11 schedule to manually make test changes and instead need automation.

Still, we should compare this ^ approach versus the https://github.com/jakartaee/platform-tck/wiki/OpenRewrite:

  1. With this ^ approach we need a prototype or at least a description of how we would handle more complex testing (e.g. EAR deployment that contains EJBs, wars and how the equivalent of running against the multiple test vehicles will be handled. One thought might be to combine testing of all the vehicles into the test directly (e.g. the test stops on the first failure) but I think that requires test changes and such changes might be hard to make in this refactoring pass.
  2. With the (current) OpenRewrite approach, we could generate new Platform TCK test sources that extend the current (SE level) tests. The current thought is to generate new test methods for each TestName + Vehicle name pair as derived from processing the https://github.com/jakartaee/platform-tck/blob/10.0.x/install/jakartaee/other/vehicle.properties#L105 information.
  3. Regardless, I think this (platform-tck/pull/1319) ^ approach for simpler web tests with some EE integration is looking like a great solution for what we might want to migrate to over time (e.g. IMO for EE 11 we should have almost the equivalent testing that we had in EE 10 but we can later migrate to a simpler/better approach when time permits).
OndroMih commented 4 weeks ago

I'm happy to keep this PR in draft and maybe never merge it. It's a demo of one of the approaches, especially for simple tests which don't require deployment in EAR. I created it only because there's no way to run EE 11 TCK against a Jakarta EE server yet as it's a work in progress.