search

jakartaee / concurrency

Eclipse Project for Concurrency Utilities
https://projects.eclipse.org/projects/ee4j.cu
Other
71 stars 39 forks source link

[TCK Challenge]: Concurrency 3.0 SignatureTest Failure in JDK 21 #392

Closed scottmarlow closed 5 months ago

scottmarlow commented 11 months ago

Specification

https://jakarta.ee/specifications/platform/10/jakarta-platform-spec-10.0#changes-in-jakarta-ee-10 only JDK11/17 are required but the intension is to support passing the TCKs on newer Java SE versions if possible (e.g. as long as needed technologies are not removed).)

Assertion

https://github.com/jakartaee/platform-tck/issues/156 was supposed to (Java version) future proof our TCKs but didn't handle new Java versions adding new methods/fields to JDK classes that the Spec API depends on..

TCK Version

3.0.2

Implementation being tested

WildFly main branch

Challenge Scenario

Something else

Full Description

2023-12-05 12:37:56,913 INFO [stdout] (default task-1) Added Superclasses or Superinterfaces 2023-12-05 12:37:56,913 INFO [stdout] (default task-1) ------------------------------------- 2023-12-05 12:37:56,913 INFO [stdout] (default task-1) 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedExecutorService: interface java.lang.AutoCloseable 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedScheduledExecutorService: interface java.lang.AutoCloseable 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) Added Methods 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) ------------- 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedExecutorService: method public void java.util.concurrent.ExecutorService.close() 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedScheduledExecutorService: method public void java.util.concurrent.ExecutorService.close() 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) ** Package 'jakarta.enterprise.concurrent' - FAILED (STATIC MODE) ** 2023-12-05 12:37:56,914 INFO [stdout] (default task-1) 2023 . . . 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) Added Superclasses or Superinterfaces 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) ------------------------------------- 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedExecutorService: interface java.lang.AutoCloseable 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedScheduledExecutorService: interface java.lang.AutoCloseable 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) Added Methods 2023-12-05 12:37:57,153 INFO [stdout] (default task-1) ------------- 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedExecutorService: method public void java.util.concurrent.ExecutorService.close() 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) jakarta.enterprise.concurrent.ManagedScheduledExecutorService: method public void java.util.concurrent.ExecutorService.close() 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) 2023-12-05 12:37:57,156 INFO [stdout] (default task-1) ** Package 'jakarta.enterprise.concurrent' - FAILED (REFLECTION MODE) **

Additional Context

I think that https://github.com/jakartaee/concurrency/issues/282 can be addressed by the same change that we would make for this one. I tested the https://github.com/jakartaee/platform-tck/pull/1206 change locally which is pretty close to a one line change in that we are just passing the additional JDK classes that have been modified to include additional methods in Java 21. I think the same change can be made here by adding the same change to the Concurrency TCK.

The Platform TCK equivalent change is shown here.

For more context, https://github.com/jakartaee/platform-tck/issues/156 is the change that we addressed for Jakarta EE 9.1 release by changing the signature test tooling to support the -IgnoreJDKClass option so we can list the JDK classes that should be ignored.

Sort of related other additional context: For Jakarta EE 11, I think we should update https://github.com/jtulach/netbeans-apitest to accept an additional parameter to request that only classes in the jakarta namespace should have their signature checked. Perhaps we could the new option something like -OnlyCheckSignaturePackage and we could stop using the -IgnoreJDKClass and instead pass -OnlyCheckSignaturePackage jakarta on the command line in the SigTestDriver.java code. The idea being that our TCKs would be more (Java version) future proof.

Is there an existing challenge for this?

KyleAure commented 11 months ago

Thanks for putting this together. I would be very interested in getting something into the signature test plugin to only check the Jakarta namespace. I think that would reduce much of the admin work it takes to keep these signature files updated after release.

scottmarlow commented 10 months ago

Thanks for putting this together. I would be very interested in getting something into the signature test plugin to only check the Jakarta namespace. I think that would reduce much of the admin work it takes to keep these signature files updated after release.

Agreed, the alternative would be as @gurunrao mentioned on https://github.com/jakartaee/platform-tck/pull/1206#issuecomment-1846605651 which is to only exclude all classes in package java..

KyleAure commented 10 months ago

@scottmarlow since this library is so integral to Jakarta EE TCK testing should eclipse manage a fork of this project so we can make these kind of changes? The base project doesn't seem to accept issues :(

scottmarlow commented 10 months ago

@scottmarlow since this library is so integral to Jakarta EE TCK testing should eclipse manage a fork of this project so we can make these kind of changes? The base project doesn't seem to accept issues :(

The current repository is used by netbeans I think and Jakarta EE 9.1. CDI/BeanValidation TCKs have also been using it for a while as well.

I'm not against Eclipse (or Apache?) forking https://github.com/jtulach/netbeans-apitest.

https://github.com/openjdk/sigtest seems to have been archived by the OpenJDK team so that isn't an option. I wonder if the various OpenJDK TCKs still depend on that or if they forked it into a different repo.

scottmarlow commented 10 months ago

@KyleAure I just noticed that https://github.com/jtulach/netbeans-apitest/blob/master/src/main/java/com/sun/tdk/signaturetest/core/PackageGroup.java#L99 already contains a package check which is called from https://github.com/jtulach/netbeans-apitest/blob/master/src/main/java/com/sun/tdk/signaturetest/SignatureTest.java#L1575 (also see comment https://github.com/jtulach/netbeans-apitest/blob/master/src/main/java/com/sun/tdk/signaturetest/SignatureTest.java#L247 which mentions that package names can be identified).

scottmarlow commented 10 months ago

Tracking issue for future proofing against further JDK changes is https://github.com/jakartaee/platform-tck/issues/1207 and as mentioned there, I wasn't able to replace the JDK class exclude list with just the package names to exclude (perhaps that excludes too much). We likely need to debug and/or add more unit tests to the netbeans-apitest project.

scottmarlow commented 10 months ago

@scottmarlow since this library is so integral to Jakarta EE TCK testing should eclipse manage a fork of this project so we can make these kind of changes? The base project doesn't seem to accept issues :(

I'm going to echo this question on the Jakarta EE Platform mailing list to see if we can get more input on the idea of forking https://github.com/jtulach/netbeans-apitest to somewhere that isn't a personal repo.

scottmarlow commented 8 months ago

@KyleAure https://github.com/eclipse-ee4j/jakartaee-tck-tools/tree/master/tools/sigtest contains our Jakarta EE fork (thanks to Scott Stark for doing that!).

Is this TCK challenge accepted? It would be helpful to have a https://download.eclipse.org/jakartaee/concurrency/3.0/ TCK release with the https://github.com/jakartaee/concurrency/pull/393 change once that is merged. Thank you!

KyleAure commented 8 months ago

@scottmarlow Thanks for your work on the sigtest plugin. Should we pull the new plugin into the 3.0.x branch and use that for the next service release? Or is the idea that we use the new plugin for 3.1.x branches and just add ignores to 3.0 as they come up for future LTS releases of java?

scottmarlow commented 8 months ago

@scottmarlow Thanks for your work on the sigtest plugin. Should we pull the new plugin into the 3.0.x branch and use that for the next service release?

I don't think we should as https://github.com/eclipse-ee4j/jakartaee-tck-tools/issues/20 is not addressed yet.

Or is the idea that we use the new plugin for 3.1.x branches and just add ignores to 3.0 as they come up for future LTS releases of java?

+1 for just adding more ignores to 3.0 as they come up for further releases of java.

Thanks!

KyleAure commented 8 months ago

@njr-11 I think we can mark this challenge accepted since it has 2 up-votes and no down-votes. Can you confirm and I can go ahead and work on getting a service release 3.0.3 published.

njr-11 commented 8 months ago

@njr-11 I think we can mark this challenge accepted since it has 2 up-votes and no down-votes. Can you confirm and I can go ahead and work on getting a service release 3.0.3 published.

Java 21 didn't exist when Concurrency 3.0 was written and was not a target Java level of the Jakarta EE 10 platform/profile specifications so this seems more optional to me, but I suppose it's fine if it can be done relatively quickly and does not interfere with the Jakarta EE 11 work for Concurrency. We will need to be producing a 3.1 RC1 and staging an equivalent copy for final very soon.

scottmarlow commented 7 months ago

Quoting from the TCK Process on how long it should take to resolve a TCK challenge:

Active Resolution

The failure to resolve a Challenge might prevent an implementation from going to market; Challenges SHOULD be given a high priority by the specification project and resolved in a timely manner. Two weeks or less SHOULD be considered the ideal period of time to resolve a challenge. Challenges may go longer as needed, but as a rule SHOULD avoid months.

If consensus cannot be reached by the specification project for a prolonged period of time, the default recommendation is to exclude the tests and address the dispute in a future revision of the specification.

I think that it is time for this challenge to be accepted or rejected so that EE 10 implementations can proceed to file compatibility results from running with Java 21. If this challenge is accepted, implementations will be able to file Java 21 compatibility results that include signature test failures by linking to this (accepted?) challenge.

Assuming this challenge is accepted, once the 3.0 TCK is updated to work with Java 21, this challenge would then be closed but will remain open until the updated TCK is released. If this challenge is rejected, then we follow a different path forward (e.g. as described in TCK Process section Rejected Challenges and Remedy.)

KyleAure commented 7 months ago

With two thumbs up on the original issue and significant time being passed I agree that this challenge should be accepted with the intention to integrate the new signature test plugin to the 3.0 service release where we can more universally ignore JDK level classes from signature testing.

scottmarlow commented 5 months ago

@KyleAure it has been a while, any ETA for when the 3.0 service release might be able to happen?

KyleAure commented 5 months ago

Sorry this has taken a back burner while we worked towards releasing Concurrency 3.1 for Jakarta 11. I'll work on doing a service release for 3.0 after the service release for 3.1.1 is complete.