Skip to content

Commit

Permalink
Integration test: Fix race condition in CoreTest
Browse files Browse the repository at this point in the history
Motivation
----------
MockConfigProvider had a race condition where the initial
ClusterConfig could be populated by a subsequent call to
accept(BucketConfig) *before* the initial config was processed,
resulting in redundant calls to Node.add/RemoveService().

This would normally be fine, since those methods are idempotent.
However, it violated the test assertion that they are called
exactly once.

Modifications
-------------
Don't publish the initial empty config; the tests don't depend on it.

Subsequent publications are implicitly serialized because the tests
verify the previous config was applied before telliing the mock config
provider to accept a new one.

Change-Id: Iab2ede125e801b6b35855a7bf1c10c106ee31d20
Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/212828
Tested-by: Build Bot <[email protected]>
Reviewed-by: David Nault <[email protected]>
  • Loading branch information
dnault committed Jul 16, 2024
1 parent 188881f commit 641c64a
Showing 1 changed file with 0 additions and 12 deletions.
12 changes: 0 additions & 12 deletions core-io/src/test/java/com/couchbase/client/core/CoreTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import java.util.Optional;

import static com.couchbase.client.test.Util.readResource;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
Expand Down Expand Up @@ -103,18 +102,12 @@ private static class MockConfigProvider {
configs.tryEmitComplete().orThrow();
return Mono.empty();
});

configs.tryEmitNext(clusterConfig).orThrow();
}

public void accept(BucketConfig bucketConfig) throws InterruptedException {
clusterConfig.setBucketConfig(bucketConfig);
logger.info("Emitting config {}", clusterConfig.allNodeAddresses());
configs.tryEmitNext(clusterConfig).orThrow();

// A sleep after emitting the config prevents it intermittently
// emitting the config twice, for reasons that are unclear.
MILLISECONDS.sleep(500);
}
}

Expand All @@ -123,7 +116,6 @@ public void accept(BucketConfig bucketConfig) throws InterruptedException {
* the difference in services and nodes is enabled.
*/
@Test
@SuppressWarnings({"unchecked"})
void addNodesAndServicesOnNewConfig() throws Exception {
MockConfigProvider mockConfigProvider = new MockConfigProvider();

Expand Down Expand Up @@ -222,7 +214,6 @@ void configureMock(Node mock, String id, String ip, int port) {
}

@Test
@SuppressWarnings("unchecked")
void addServicesOnNewConfig() throws Exception {
MockConfigProvider mockConfigProvider = new MockConfigProvider();

Expand Down Expand Up @@ -307,7 +298,6 @@ protected Node createNode(final NodeIdentifier target) {
}

@Test
@SuppressWarnings("unchecked")
void removeNodesAndServicesOnNewConfig() throws Exception {
MockConfigProvider mockConfigProvider = new MockConfigProvider();

Expand Down Expand Up @@ -377,7 +367,6 @@ protected Node createNode(final NodeIdentifier target) {
}

@Test
@SuppressWarnings("unchecked")
void removesNodeIfNotPresentInConfigAnymore() throws Exception {
MockConfigProvider mockConfigProvider = new MockConfigProvider();

Expand Down Expand Up @@ -450,7 +439,6 @@ protected Node createNode(final NodeIdentifier target) {
* the node is identified by a tuple of hostname and manager port, and this should work.
*/
@Test
@SuppressWarnings("unchecked")
void addsSecondNodeIfBothSameHostname() throws Exception {
MockConfigProvider mockConfigProvider = new MockConfigProvider();

Expand Down

0 comments on commit 641c64a

Please sign in to comment.