Summary
RequestDistributorTest.testSimpleRequest in connector-framework-rpc intermittently fails on CI with a stale entry left in the client's remote-request tracking map, unrelated to the PR under test.
Observed failure
Seen on PR #130 (a CI-workflow-only change, no Java diff), job build-maven (ubuntu-latest, 11):
[ERROR] org.forgerock.openicf.common.rpc.RequestDistributorTest.testSimpleRequest -- Time elapsed: 0.003 s <<< FAILURE!
java.lang.AssertionError: expected [true] but found [false]
at org.testng.Assert.assertTrue(Assert.java:52)
at org.forgerock.openicf.common.rpc.RequestDistributorTest.testSimpleRequest(RequestDistributorTest.java:147)
Line 147 is Assert.assertTrue(client.getRemoteRequests().isEmpty()), checked right at the start of the test — before it submits anything itself.
Root cause hypothesis
Several @Test methods in this file (testSimpleRequest, testCallbackRequest, testFailedRequest, …) declare only dependsOnMethods = { "testNoConnectionRequest" }. TestNG guarantees ordering against that one method, but not ordering relative to each other. Each of these methods asserts the shared client/server TestConnectionGroup instance's request maps are empty on entry, assuming the previous test's finally { connection.close(); } fully drained remoteRequests/localRequests (via async promise completion/cancellation) before returning. When that async cleanup hasn't finished yet, the next test's entry assertion races and fails.
This is a known-flaky area: commit d7d79f7 ("Try to resolve the unstable unit test") already attempted a fix here and evidently didn't fully close the race.
Possible relation to PR #127
PR #127 ([#124] "Create the RemoteRequest promise at construction so an unsent request can be cancelled") touches exactly the RemoteRequest/RemoteConnectionGroup lifecycle implicated here, and adds RemoteRequestCancelBeforeSendTest. It's worth checking whether that change also resolves (or at least improves) this flake as a side effect.
Suggested next step
Either give each dependent test its own isolated client/server instance (avoid shared mutable state across test methods), or make cleanup in close()/promise cancellation synchronous/awaited before the next test method starts.
Summary
RequestDistributorTest.testSimpleRequestinconnector-framework-rpcintermittently fails on CI with a stale entry left in the client's remote-request tracking map, unrelated to the PR under test.Observed failure
Seen on PR #130 (a CI-workflow-only change, no Java diff), job
build-maven (ubuntu-latest, 11):Line 147 is
Assert.assertTrue(client.getRemoteRequests().isEmpty()), checked right at the start of the test — before it submits anything itself.Root cause hypothesis
Several
@Testmethods in this file (testSimpleRequest,testCallbackRequest,testFailedRequest, …) declare onlydependsOnMethods = { "testNoConnectionRequest" }. TestNG guarantees ordering against that one method, but not ordering relative to each other. Each of these methods asserts the sharedclient/serverTestConnectionGroupinstance's request maps are empty on entry, assuming the previous test'sfinally { connection.close(); }fully drainedremoteRequests/localRequests(via async promise completion/cancellation) before returning. When that async cleanup hasn't finished yet, the next test's entry assertion races and fails.This is a known-flaky area: commit d7d79f7 ("Try to resolve the unstable unit test") already attempted a fix here and evidently didn't fully close the race.
Possible relation to PR #127
PR #127 ([#124] "Create the RemoteRequest promise at construction so an unsent request can be cancelled") touches exactly the
RemoteRequest/RemoteConnectionGrouplifecycle implicated here, and addsRemoteRequestCancelBeforeSendTest. It's worth checking whether that change also resolves (or at least improves) this flake as a side effect.Suggested next step
Either give each dependent test its own isolated
client/serverinstance (avoid shared mutable state across test methods), or make cleanup inclose()/promise cancellation synchronous/awaited before the next test method starts.