| Re: [jaxrs-dev] [External] : Rx TCK Tests |
|
Hi James,
> race
condition to me to assume the future is done at this stage
It is
doing the opposite.
The test checks that the future is NOT done, yet, i.e. that the rx code is nonblocking, running the HTTP query in a new thread, passing the main thread directly to Future.isDone().
GitHub Actions seems to be doing some funny stuff with threads though. The code might be done better, adding some countdown latch now when the client is run one the same JVM as the server, unlike the EE 8 days.
-- Jan
From: jaxrs-dev <jaxrs-dev-bounces@xxxxxxxxxxx> on behalf of James Perkins <jperkins@xxxxxxxxxx>
Sent: Thursday, March 24, 2022 9:24 PM To: jaxrs-dev <jaxrs-dev@xxxxxxxxxxx> Subject: [External] : [jaxrs-dev] Rx TCK Tests Hello All,
I'm curious if anyone has some background on tests like [1]. It creates a CompletionStageRxInvoker, starts a request and converts the CompletationStage to a Future. All that seems fine, except when it gets to [2] where it checks if the future is done.
It seems like a bit of a race condition to me to assume the future is done at this stage. While I've seen no issues running this locally or on most CI environments I've run it on, I do see issues on GitHub Actions which is failing several of the tests like
this. It's definitely a timing issue because they don't all consistently fail, but it always fails at least one of them because the future is not yet done.
My assumption is the Future.isDone() check is to avoid the possible hang on Future.get(). However, it seems we could easily change that to something like Future.get(configurableTimeout, TimeUnit.SECONDS). Does this seem reasonable? I'm happy to contribute a
patch if it seems reasonable.
Just to note too I absolutely do not think this should block any current 3.1 efforts :) The tests have been like this for a long time and for me it's ONLY failing on GitHub Actions.
Thanks in advance!
|