concurrency icon indicating copy to clipboard operation
concurrency copied to clipboard

First pass at removing remote ejb interface usage

Open starksm64 opened this issue 2 years ago • 12 comments

Addresses #244

Signed-off-by: Scott M Stark [email protected]

starksm64 avatar Jun 28 '22 17:06 starksm64

If this particular place is switched to web.xml, then that coverage is lost entirely.

I guess the best move would be to add that particular test to a "javaee-full" group, so that the web-profile run can add that to an '<excludedGroups>'.

arjantijms avatar Jul 01 '22 16:07 arjantijms

Right, it would have to be excludable from the web profile to remain in its current form. I'll add that group to the test instead.

starksm64 avatar Jul 01 '22 17:07 starksm64

Unfortunately, converting ears to wars has introduced regressions in the full profile run and these exist in the web profile run as well. This is what I see when running that tck runner bundled with the glassfish repo against the tck in this pr.

Full platform:

[ERROR] Failures: 
[ERROR]   ContextPropagationTests>Arquillian.run:138->testContextServiceDefinitionFromEJBAllAttributes:212->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ContextPropagationTests>Arquillian.run:138->testContextServiceDefinitionFromEJBDefaults:232->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ContextPropagationTests>Arquillian.run:138->testJNDIContextAndCreateProxyInEJB:149->TestClient.assertStringInResponse:171->ArquillianTests.assertTrue:108 testJNDIContextAndCreateProxyInEJBfailed to get correct result. expected [true] but found [false]
[ERROR]   ManagedExecutorDefinitionTests>Arquillian.run:138->testCopyCompletableFutureEJB:116->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedExecutorDefinitionTests>Arquillian.run:138->testIncompleteFutureEJB:127->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   InheritedAPITests>Arquillian.run:138->testApiScheduleAtFixedRate:122 » ArquillianProxy jakarta.ejb.EJBException : null [Proxied because : Original exception caused: class java.lang.ClassNotFoundException: jakarta.ejb.EJBException]
[ERROR]   ManagedScheduledExecutorDefinitionTests>Arquillian.run:138->testIncompleteFutureMSE_EJB:112->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedScheduledExecutorDefinitionTests>Arquillian.run:138->testManagedScheduledExecutorDefinitionAllAttributes_EJB:123->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedScheduledExecutorDefinitionTests>Arquillian.run:138->testManagedScheduledExecutorDefinitionDefaults_EJB:134->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[INFO] 
[ERROR] Tests run: 149, Failures: 9, Errors: 0, Skipped: 0

Web profile with eefull group excluded:

[ERROR] Failures: 
[ERROR]   ContextPropagationTests>Arquillian.run:138->testContextServiceDefinitionFromEJBAllAttributes:212->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ContextPropagationTests>Arquillian.run:138->testContextServiceDefinitionFromEJBDefaults:232->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ContextPropagationTests>Arquillian.run:138->testJNDIContextAndCreateProxyInEJB:149->TestClient.assertStringInResponse:171->ArquillianTests.assertTrue:108 testJNDIContextAndCreateProxyInEJBfailed to get correct result. expected [true] but found [false]
[ERROR]   ManagedExecutorDefinitionTests>Arquillian.run:138->testCopyCompletableFutureEJB:116->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedExecutorDefinitionTests>Arquillian.run:138->testIncompleteFutureEJB:127->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedScheduledExecutorDefinitionTests>Arquillian.run:138->testIncompleteFutureMSE_EJB:112->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedScheduledExecutorDefinitionTests>Arquillian.run:138->testManagedScheduledExecutorDefinitionAllAttributes_EJB:123->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[ERROR]   ManagedScheduledExecutorDefinitionTests>Arquillian.run:138->testManagedScheduledExecutorDefinitionDefaults_EJB:134->TestClient.runTest:70->TestClient.assertSuccessfulURLResponse:143->ArquillianTests.assertTrue:108 Output did not contain successful message: SUCCESS expected [true] but found [false]
[INFO] 
[ERROR] Tests run: 146, Failures: 8, Errors: 0, Skipped: 0

starksm64 avatar Jul 04 '22 03:07 starksm64

Can't we just create an Arquillian extension that repackages EAR into WAR (where it makes sense)? Where it doesn't make sense (like when the test relies on application.xml), just exclude the test from web profile.

Obviously, remote EJBs would have to be refactored to local ones anyway.

This could allow running the tests with EAR in Full profile without any change, and with WAR for a Web Profile. We've done that before with some MicroProfile TCK, where tests relied on WAR deployments but an implementation could only ran plain JAR files.

OndroMih avatar Jul 05 '22 14:07 OndroMih

@OndroMih would that help avoid the 8 remaining test failures?

scottmarlow avatar Jul 05 '22 15:07 scottmarlow

The problem seems to be that portable jndi names are changing as a function of the deployment packaging. There are hard coded jndi name lookups that cannot easily be changed. It seems to me that the concurrency tck needs to be refactored to first address what is relevant in a web profile, and then have additional tests as required that target the full platform.

starksm64 avatar Jul 05 '22 18:07 starksm64

I attempted to update https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run_hacktest/ to build this pull request and run the TCK using it.

I updated the jakarta-concurrency-tck-glassfish-run_hacktest to hack the Platform TCK GlassFish Concurrency TCK runner suite.xml file by inserting one line:

<groups> <run> <exclude name="eefull" /> </run>   </groups>

If I run jakarta-concurrency-tck-glassfish-run_hacktest against web, I see 17 failures.

If I run full, no changes are made to the suite.xml and we seem to just pass all tests.

The jakarta-concurrency-tck-glassfish-run_hacktest job isn't how we test GlassFish so the job isn't perfect. So, I'm not sure if the test results are valid.

scottmarlow avatar Jul 05 '22 22:07 scottmarlow

Since I see 17 test failures for web instead of 8 failures, I assume I am doing something different and wrong.

scottmarlow avatar Jul 05 '22 22:07 scottmarlow

The new tck is being built, but it is a 3.0.1-SNAPSHOT that is not installed because the run of the artifact-install.sh script is passed 3.0.1 as the version. The version also needs to be updated in the run pom. I had created two jobs to test the PR:

https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-build-test/ Builds the 3.0.1-SNAPSHOT concurrency tck from https://github.com/jakartaredhat/concurrency.git repository and stages to https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee10/staged/epl/

https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run-test/ Runs the tck against the 3.0.1-SNAPSHOT downloaded from https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee10/staged/epl/concurrency-tck-3.0.1-SNAPSHOT-dist.zip

The run I just did is showing 9 failures on web profile: https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-concurrency-tck-glassfish-run-test/9/

On Jul 5, 2022 at 5:24:14 PM, Scott Marlow @.***> wrote:

Since I see 17 test failures for web instead of 8 failures, I assume I am doing something different and wrong.

— Reply to this email directly, view it on GitHub https://github.com/jakartaee/concurrency/pull/245#issuecomment-1175560597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACRDMVBJQ73Y5A7WGN2N73VSSYY5ANCNFSM52C7O3ZA . You are receiving this because you authored the thread.Message ID: @.***>

starksm64 avatar Jul 06 '22 14:07 starksm64

I've tried out the 3.0.1-SNAPSHOT TCK against Open Liberty and see deployment errors with most of the same applications that Scott mentioned. I get a more useful looking error out of OL, though, that can perhaps help figure out what updates need to be made:

CWNEN0054E: The ContextPropagationTests_web.war bean in the ContextPropagationTests_web.war module of the
ContextPropagationTests_web application has conflicting configuration data in source code annotations. Conflicting
cleared attribute values exist for multiple @ContextServiceDefinition annotations with the same name attribute value :
java:module/concurrent/ContextB. The conflicting cleared attribute values are [Transaction] and [IntContext].

For reference, the conflicting definitions can be found here: https://github.com/jakartaee/concurrency/blob/master/tck/src/main/java/ee/jakarta/tck/concurrent/spec/ContextService/contextPropagate/ContextServiceDefinitionBean.java#L38 https://github.com/jakartaee/concurrency/blob/master/tck/src/main/java/ee/jakarta/tck/concurrent/spec/ContextService/contextPropagate/ContextServiceDefinitionServlet.java#L63

brideck avatar Jul 06 '22 16:07 brideck

I've tried out the 3.0.1-SNAPSHOT TCK against Open Liberty and see deployment errors with most of the same applications that Scott mentioned. I get a more useful looking error out of OL, though, that can perhaps help figure out what updates need to be made:

CWNEN0054E: The ContextPropagationTests_web.war bean in the ContextPropagationTests_web.war module of the
ContextPropagationTests_web application has conflicting configuration data in source code annotations. Conflicting
cleared attribute values exist for multiple @ContextServiceDefinition annotations with the same name attribute value :
java:module/concurrent/ContextB. The conflicting cleared attribute values are [Transaction] and [IntContext].

For reference, the conflicting definitions can be found here: https://github.com/jakartaee/concurrency/blob/master/tck/src/main/java/ee/jakarta/tck/concurrent/spec/ContextService/contextPropagate/ContextServiceDefinitionBean.java#L38 https://github.com/jakartaee/concurrency/blob/master/tck/src/main/java/ee/jakarta/tck/concurrent/spec/ContextService/contextPropagate/ContextServiceDefinitionServlet.java#L63

Brian, thanks for trying that out and spotting that error. That's a TCK test that intentionally defines the same java:module name with different ContextService configurations in 2 different modules, verifying that each module abides by its respective configuration. It isn't valid to repackage that test into a single module, because there would be a conflict (as observed) which is a different behavior. Is this just accidental overreach of attempting to replace remote EJBs which ni this particular case should have been left as is? This is a newer test of 3.0 function and I don't think remote EJB was used for it.

njr-11 avatar Jul 06 '22 16:07 njr-11

See also https://github.com/jakartaee/concurrency/pull/250

arjantijms avatar Jul 25 '22 11:07 arjantijms

closing this as obsolete

smillidge avatar Dec 18 '22 21:12 smillidge