Skip to content

Commit

Permalink
Fix Flaky Tests in NettyCommTest, CompileProxyTest, DeltaStreamTest (#…
Browse files Browse the repository at this point in the history 
…3624)

* Fix Flaky Tests in NettyCommTest, CompileProxyTest

- NettyCommTest:
Make bootstrapServer() wait for the server to be up and running before checking for ping messages.

Move all combinations of ciphers and keystore args into separate methods to enable threads clean up after each test.

- CompileProxyTest:
Increase timeout for executeScheduled as it takes more than 1 second to  execute the scheduled the task.

* Fix DeltaStreamTest.concurrencyTest() timeout

Increase `done` latch's wait timeout to 10 seconds
  • Loading branch information
@chetangudisagar
chetangudisagar committed May 22, 2023
1 parent 9f8422e commit f89653a
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 11 deletions.
56 changes: 50 additions & 6 deletions test/src/test/java/org/corfudb/runtime/clients/NettyCommTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,10 @@ public void testTlsUpdateServerTrust() throws Exception {

clientRouter.getConnectionFuture().join();
assertThat(getBaseClient(clientRouter).pingSync()).isTrue();
clientRouter.stop();
clientRouter.close();

serverData.shutdownServer();
serverData.serverContext.close();
}

@Test
Expand Down Expand Up @@ -432,9 +433,10 @@ public void testTlsUpdateClientTrust() throws Exception {

clientRouter.getConnectionFuture().join();
assertThat(getBaseClient(clientRouter).pingSync()).isTrue();
clientRouter.stop();
clientRouter.close();

serverData.shutdownServer();
serverData.serverContext.close();
}

/**
Expand All @@ -444,15 +446,30 @@ public void testTlsUpdateClientTrust() throws Exception {
* @throws Exception Any Exception thrown during the test
*/
@Test
public void testTlsCipherRSA() throws Exception {
public void testTlsCipherRsaKeyStoreRsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256.name(), KeyStoreType.RSA,
true, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256);
}
@Test
public void testTlsCipherRsaKeyStoreEcdsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256.name(), KeyStoreType.ECDSA,
false, null);
}

@Test
public void testTlsCipherRsaKeyStoreRsaEcdsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256.name(), KeyStoreType.RSA_ECDSA,
true, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256);
}

@Test
public void testTlsCipherRsaKeyStoreRsaRsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256.name(), KeyStoreType.RSA_RSA,
true, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256);
}

@Test
public void testTlsCipherRsaKeyStoreEcdsaEcdsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256.name(), KeyStoreType.ECDSA_ECDSA,
false, null);
}
Expand All @@ -464,12 +481,19 @@ public void testTlsCipherRSA() throws Exception {
* @throws Exception Any Exception thrown during the test
*/
@Test
public void testTlsCipherEcdsa() throws Exception {
public void testTlsCipherEcdsaKeyStoreRsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.name(), KeyStoreType.RSA,
false, null);
}

@Test
public void testTlsCipherEcdsaKeyStoreEcdsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.name(), KeyStoreType.ECDSA,
true, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
}

@Test
public void testTlsCipherEcdsaKeyStoreRsaEcdsa() throws Exception {
// When OpenSsl is the SSL provider
// TLS fails sometimes, depending on the environment to find a Cipher when both RSA and ECDSA are in the trust store
// javax.net.ssl.SSLHandshakeException: error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher
Expand All @@ -485,10 +509,16 @@ public void testTlsCipherEcdsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.name(), KeyStoreType.RSA_ECDSA,
true, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
}
}

@Test
public void testTlsCipherEcdsaKeyStoreRsaRsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.name(), KeyStoreType.RSA_RSA,
false, null);
}

@Test
public void testTlsCipherEcdsaKeystoreEcdsaEcdsa() throws Exception {
tlsCipherTestHelper(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.name(), KeyStoreType.ECDSA_ECDSA,
true, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
}
Expand All @@ -500,13 +530,19 @@ public void testTlsCipherEcdsa() throws Exception {
* @throws Exception Any Exception thrown during the test
*/
@Test
public void testTlsCipherRsaAndEcdsa() throws Exception {
public void testTlsCipherRsaAndEcdsaKeyStoreRsa() throws Exception {
tlsCipherTestHelper(ConfigParamsHelper.getTlsCiphersCSV(), KeyStoreType.RSA,
true, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256);
}

@Test
public void testTlsCipherRsaAndEcdsaKeyStoreEcdsa() throws Exception {
tlsCipherTestHelper(ConfigParamsHelper.getTlsCiphersCSV(), KeyStoreType.ECDSA,
true, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
}

@Test
public void testTlsCipherRsaAndEcdsaKeyStoreRsaEcdsa() throws Exception {
if (OpenSsl.isAvailable()) {
// when OpenSsl is the SSL provider
// Picks RSA Cipher TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 when both are given
Expand All @@ -518,10 +554,16 @@ public void testTlsCipherRsaAndEcdsa() throws Exception {
tlsCipherTestHelper(ConfigParamsHelper.getTlsCiphersCSV(), KeyStoreType.RSA_ECDSA,
true, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
}
}

@Test
public void testTlsCipherRsaAndEcdsaKeyStoreRsaRsa() throws Exception {
tlsCipherTestHelper(ConfigParamsHelper.getTlsCiphersCSV(), KeyStoreType.RSA_RSA,
true, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256);
}

@Test
public void testTlsCipherRsaAndEcdsaKeyStoreEcdsaEcdsa() throws Exception {
tlsCipherTestHelper(ConfigParamsHelper.getTlsCiphersCSV(), KeyStoreType.ECDSA_ECDSA,
true, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
}
Expand Down Expand Up @@ -607,8 +649,9 @@ private void runWithBaseServer(
} finally {
try {
if (ncr != null) {
ncr.stop();
ncr.close();
}
d.serverContext.close();
} catch (Exception ex) {
log.warn("Error shutting down client...", ex);
}
Expand Down Expand Up @@ -682,6 +725,7 @@ void bootstrapServer() {
nsr,
address,
Integer.parseInt((String) serverContext.getServerConfig().get("<port>")));
f.syncUninterruptibly();
}

void shutdownServer() {
Expand Down
2 changes: 1 addition & 1 deletion test/src/test/java/org/corfudb/runtime/collections/SMRMapEntrySetTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public void manipulateSetsConcurrent() throws Exception {
assertThat(keys.contains(0L)).isFalse();
});

executeScheduled(PARAMETERS.CONCURRENCY_SOME, PARAMETERS.TIMEOUT_SHORT);
executeScheduled(PARAMETERS.CONCURRENCY_SOME, PARAMETERS.TIMEOUT_NORMAL);

}
}
3 changes: 2 additions & 1 deletion test/src/test/java/org/corfudb/runtime/collections/streaming/DeltaStreamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.corfudb.AbstractCorfuTest.PARAMETERS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -309,7 +310,7 @@ public void concurrencyTest() throws Exception {

producer.setName("producer");
producer.start();
done.await(1, TimeUnit.SECONDS);
done.await(PARAMETERS.TIMEOUT_NORMAL.getSeconds(), TimeUnit.SECONDS);

assertThat(consumed.size()).isEqualTo(numToProduce);
for (int x = 0; x < numToProduce; x++) {
Expand Down
6 changes: 3 additions & 3 deletions test/src/test/java/org/corfudb/runtime/object/CompileProxyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public void testObjectCounterCASConcurrency() throws Exception {
if (sharedCounter.CAS(INITIAL, t) == INITIAL)
casSucceeded.incrementAndGet();
});
executeScheduled(concurrency, PARAMETERS.TIMEOUT_SHORT);
executeScheduled(concurrency, PARAMETERS.TIMEOUT_NORMAL);

// check that exactly one CAS succeeded
assertThat(sharedCounter.getValue())
Expand Down Expand Up @@ -311,7 +311,7 @@ public void testCorfuMapConcurrency() throws Exception {
scheduleConcurrently(concurrency, t -> {
map.insert(Integer.toString(t), "world");
});
executeScheduled(concurrency, PARAMETERS.TIMEOUT_SHORT);
executeScheduled(concurrency, PARAMETERS.TIMEOUT_NORMAL);

// check that map contains keys with every thread index
for (int i = 0; i < concurrency; i++)
Expand Down Expand Up @@ -376,7 +376,7 @@ public void testObjectCompoundWriteConcurrency() throws Exception {
scheduleConcurrently(concurrency, t -> {
sharedCorfuCompound.set(sharedCorfuCompound.new Inner("A"+t, "B"+t), t);
});
executeScheduled(concurrency, PARAMETERS.TIMEOUT_SHORT);
executeScheduled(concurrency, PARAMETERS.TIMEOUT_NORMAL);

// sanity checks on the final value of sharedCorfuCompound
assertThat(sharedCorfuCompound.getID())
Expand Down

0 comments on commit f89653a

Please sign in to comment.