jakartaee / concurrency

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

Removing Tests of Context Switch from Asynchronous Tests #497

Closed aubi closed 4 months ago

aubi commented 4 months ago

I removed tests of context switch, which tested, that the context capture happened during jndi lookup. The test later setup number 22 and the test checks, if asynchronous method (running via scheduler) returns 0.

The code:

ManagedExecutorDefinitionFullTests.testScheduledAsynchIgnoresMaxAsync() {
...
            IntContext.set(22); //Context should be cleared
            CompletableFuture<Integer> future = reqBean.scheduledEvery3Seconds(1, counter);
...
            assertEquals(Integer.valueOf(0), future.get(MAX_WAIT_SECONDS, TimeUnit.SECONDS),
                    "ManagedScheduledExecutorService with maxAsync=4 must be able to run scheduled async methods concurrently.");

reqBean.scheduledEvery3Seconds runs only once and returns IntContext: future.complete(IntContext.get())

  1. There is no explicit setup of the IntContext value to test.
  2. This test checks just the context switch, not asynch methods running concurrently as it says in the message. Although it is called schedule, it runs only once.

The discussion, if the context capture should happen on JNDI lookup or when the thread is created, is still open: https://github.com/jakartaee/concurrency/issues/253

aubi commented 4 months ago

I see your point, Nathan. Your suggestions look good, I'll send a commit in a second.

aubi commented 4 months ago

@njr-11 If you will agree with the changes, I'll merge the commits, so there is just one commit, not two doing remove/readd.