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

TCK Challenge Managed(Scheduled)ExecutorDefinitionTests.testCompletedFuture(MSE) Tests #224

Closed
aubi opened this issue May 13, 2022 · 3 comments
Labels
accepted Accepted certification request challenge TCK challenge
Milestone

Comments

@aubi
Copy link
Contributor

aubi commented May 13, 2022

Specification:

Jakarta Concurrency 3.0.0

Challenged test(s):

ee.jakarta.tck.concurrent.spec.ManagedExecutorService.resourcedef.ManagedExecutorDefinitionTests#testCompletedFuture

ee.jakarta.tck.concurrent.spec.ManagedScheduledExecutorService.resourcedef.ManagedScheduledExecutorDefinitionTests#testCompletedFutureMSE

TCK version:

3.0.0

Tested implementation:

Payara 6 (branch) with Concurrent-RI (branch)

Description

Test ManagedExecutorDefinitionTests/ManagedExecutorDefinitionServlet.testCompletedFuture uses "java:module/concurrent/ExecutorB" ManagedExecutorService, which is configured to use this context:

@ContextServiceDefinition(name = "java:module/concurrent/ContextB",
                          cleared = TRANSACTION,
                          unchanged = { APPLICATION, IntContext.NAME },
                          propagated = ALL_REMAINING)

E.g. the implementation must keep APPLICATION (JNDI search) unchanged during the thenAcceptAsync() call.

This test expects, that the thread's context doesn't recognize java:module JNDI lookup. As I understand, Open Liberty starts threads with "empty contexts", so JNDI calls fail and TCK test succeeds.

The same happens in ee.jakarta.tck.concurrent.spec.ManagedScheduledExecutorService.resourcedef.ManagedScheduledExecutorDefinitionService#testCompletedFutureMSE

A message from Nathan:

It looks like a mistake in the test case where it is accidentally testing the ‘unchanged’ setting as though it were ‘cleared’. The unchanged setting leaves the context type alone, so whatever the context was on the thread before it must remain. The mistake went unnoticed on Open Liberty because the state of the thread happens to already match the test assertion. This one should definitely be challenged for the mistaken assertion.

@smillidge smillidge added the challenge TCK challenge label May 14, 2022
@scottmarlow
Copy link

As per TCK Process this challenge should be accepted via lazy consensus.

@aubi
Copy link
Contributor Author

aubi commented Jun 17, 2022

delete -> The fix for this challenge is here: #212
It removes changes of context between JNDI lookup and newThread, so both ways of understanding will be accepted.

Of course, we should agree on a final, more exact behavior, write it to doc and return the detailed tests.<-- delete

There is no fix - either we should use the cleared for APPLICATION or we should call the unchanged APPLICATION from well-defined thread.

@smillidge smillidge added the accepted Accepted certification request label Jun 27, 2022
@smillidge
Copy link
Contributor

I accepted the challenge and will disable the test in the 3.0.1 TCK service release

@smillidge smillidge added appealed-challenge TCK challenge was appealed accepted Accepted certification request and removed accepted Accepted certification request appealed-challenge TCK challenge was appealed labels Jun 27, 2022
@smillidge smillidge added this to the 3.0.1 milestone Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted certification request challenge TCK challenge
Projects
None yet
Development

No branches or pull requests

3 participants