ConcurrentModificationException in TCK 10

[dmatej]

Hi, when testing TCK10 against GF7.1.0-SNAPSHOT on JDK17 one test failed, but the error is not in GF. I have found that it was already fixed in TCK11 and 12, would it be possible to backport it also to TCK10?

Actually I already made that, I am very sorry I did not create this issue first as it is requested by the TCK process: https://jakarta.ee/committees/specification/tckprocess/.

• If you would accept this soon, everything would be resolved for now.
• For the case this issue would be refused, I already created this PR which would revert the change: Revert "Defend against intermittent EJB failures" committed and merged in #2396 without correct process platform-tck#2397

The original fix for TCK11: jakartaee/platform-tck#2397
The PR I merged so I broke rules: jakartaee/platform-tck#2396

Thanks to @scottmarlow for the guidance.

[scottmarlow]

@dmatej as of yet correcting intermittent failures is not covered by the TCK process document https://jakarta.ee/committees/specification/tckprocess (see Accepted Challenges). Unless someone has ideas for improving the TCK Process document we should revert the fix for EE 10. Note that a recent TCK Process change that talks about test changes is jakartaee/jakarta.ee@f96a44f

I suppose that the group of intermittent failures probably also contains the recent Apache download site failures which seem hard to deal with.

[OndroMih]

I agree with @scottmarlow , the test should be challenged for EE 10 and disabled in a patch release, while it's already fixed in EE 11. Unfortunately, according to the current process, released tests cannot be fixed or modified in any way, only disabled.

[OndroMih]

So, I believe that we should do this:

• merge Revert "Defend against intermittent EJB failures" committed and merged in #2396 without correct process platform-tck#2397 to revert the fix in EE 10
• turn this issue into a challenge or close it and raise another issue as a challenge
• after the challenge is accepted, the test would be disabled in EE 10

If we want to run the fixed test against GlassFish 7.1, we would have to copy it into the GlassFish test suite and fix it there.

[jhanders34]

Unfortunately, according to the current process, released tests cannot be fixed or modified in any way, only disabled.

That is not completely accurate. The TCK process was updated to include the following which would allow for updating tests in a way to fix things that are test problems, but do not change the nature of the test in any way or if changing the test, it is to expand the test, but never to limit it.

As another alternative to excluding a challenged test, it may be possible to adjust the test validation logic to “expand” the validation check. E.g. if a test expects a value “A1” for a specific variable “x”, but a challenge is raised arguing that the specification language actually allows for either values “A1” and “A2” (but no other values) to be valid values for “x”, then it could be a valid course of action to modify the challenged test to allow either “A1” OR “A2” for “x”.

Since this line of thinking might be applied to cases that aren’t quite as straightforward as this example, care should be taken when using this approach. A particular danger is that an implementation that has already demonstrated compliance before the challenge was raised might actually not pass the new, modified test.

To limit the confusion and additional work such a scenario would cause, if there is already at least one certified compatible implementation before the challenge, the new, modified TCK should be run against at least one such implementation (and ideally all of them) before the changes are published, released, and finalized.

If such a change were released, and it was later found to cause a previously-certified implementation to fail the new, modified test, then excluding the test would likely be the only option, and this would require yet another, additional service release.

That all being said, it is allowed to fix a broken test for things like concurrency issues where a test is not using thread safe ways of doing things that are test problems and the change does not change how the function is being measured.

[OndroMih]

@jhanders34 , thank you for the info, I didn't know that this is allowed. So then it seems we can fix the test and don't need to revert the change, right @scottmarlow ?

[scottmarlow]

@jhanders34 , thank you for the info, I didn't know that this is allowed. So then it seems we can fix the test and don't need to revert the change, right @scottmarlow ?

With each TCK challenge that we accept a test change for we are taking a chance that we could lead to an already compliant EE implementation to fail the updated TCK test. In such a situation we would have to then exclude the relevant test in a further service release.

I suppose that intermittent failures that are caused by faulty code should also be considered for TCK challenges. So sure, please do create a TCK challenge for the failure that we see on EE 10 with the test failure as well that we currently see that WOULD be addressed by the already merged change.

[dmatej]

Where should I create another issue? I thought this is the one - note that I am not allowed to add labels to issues here. Somebody else has to do that.

[scottmarlow]

Where should I create another issue? I thought this is the one - note that I am not allowed to add labels to issues here. Somebody else has to do that.

https://github.com/jakartaee/platform-tck is where the challenge would be created for the Jakarta EE 10 Platform TCK. I think that this one can be closed.

jakartaee/platform-tck#2322 has one of the test failure examples in it.