Skip to content

Commit

Permalink
Improve flaky test execution (#880)
Browse files Browse the repository at this point in the history
Co-authored-by: Bryan Burkholder <[email protected]>
  • Loading branch information
bryanlb and bryanlb authored Apr 24, 2024
1 parent 1482898 commit 56fdd33
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 43 deletions.
4 changes: 2 additions & 2 deletions astra/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<opensearch.version>2.11.1</opensearch.version>
<curator.version>5.6.0</curator.version>
<log4j.version>2.23.1</log4j.version>
<aws.sdk.version>2.25.31</aws.sdk.version>
<aws.sdk.version>2.25.36</aws.sdk.version>
<error.prone.version>2.23.0</error.prone.version>
<junit.jupiter.version>5.10.2</junit.jupiter.version>
</properties>
Expand Down Expand Up @@ -332,7 +332,7 @@
<dependency>
<groupId>software.amazon.awssdk.crt</groupId>
<artifactId>aws-crt</artifactId>
<version>0.29.14</version>
<version>0.29.18</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
Expand Down
95 changes: 57 additions & 38 deletions astra/src/test/java/com/slack/astra/blobfs/s3/S3CrtBlobFsTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.slack.astra.blobfs.s3;

import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -223,15 +224,22 @@ public void testDeleteFolder() throws Exception {

s3BlobFs.delete(URI.create(String.format(FILE_FORMAT, SCHEME, bucket, folderName)), true);

ListObjectsV2Response listObjectsV2Response =
s3Client.listObjectsV2(S3TestUtils.getListObjectRequest(bucket, "", true)).get();
String[] actualResponse =
listObjectsV2Response.contents().stream()
.map(S3Object::key)
.filter(x -> x.contains("delete-2"))
.toArray(String[]::new);

assertEquals(0, actualResponse.length);
// await ignoreExceptions is a workaround due to //
// https://github.com/aws/aws-sdk-java-v2/issues/3658
await()
.ignoreExceptions()
.until(
() ->
s3Client
.listObjectsV2(S3TestUtils.getListObjectRequest(bucket, "", true))
.get()
.contents()
.stream()
.map(S3Object::key)
.filter(x -> x.contains("delete-2"))
.toArray(String[]::new)
.length
== 0);
}

@Test
Expand Down Expand Up @@ -278,35 +286,46 @@ public void testExists() throws Exception {
createEmptyFile(folderName, fileName);
}

boolean bucketExists = s3BlobFs.exists(URI.create(String.format(DIR_FORMAT, SCHEME, bucket)));
boolean dirExists =
s3BlobFs.exists(URI.create(String.format(FILE_FORMAT, SCHEME, bucket, folder)));
boolean childDirExists =
s3BlobFs.exists(
URI.create(
String.format(FILE_FORMAT, SCHEME, bucket, folder + DELIMITER + childFolder)));
boolean fileExists =
s3BlobFs.exists(
URI.create(
String.format(
FILE_FORMAT,
SCHEME,
bucket,
folder + DELIMITER + childFolder + DELIMITER + "a-ex.txt")));
boolean fileNotExists =
s3BlobFs.exists(
URI.create(
String.format(
FILE_FORMAT,
SCHEME,
bucket,
folder + DELIMITER + childFolder + DELIMITER + "d-ex.txt")));

assertTrue(bucketExists);
assertTrue(dirExists);
assertTrue(childDirExists);
assertTrue(fileExists);
assertFalse(fileNotExists);
// await ignoreExceptions is a workaround due to //
// https://github.com/aws/aws-sdk-java-v2/issues/3658
await()
.ignoreExceptions()
.until(() -> s3BlobFs.exists(URI.create(String.format(DIR_FORMAT, SCHEME, bucket))));
await()
.ignoreExceptions()
.until(
() -> s3BlobFs.exists(URI.create(String.format(FILE_FORMAT, SCHEME, bucket, folder))));
await()
.ignoreExceptions()
.until(
() ->
s3BlobFs.exists(
URI.create(
String.format(
FILE_FORMAT, SCHEME, bucket, folder + DELIMITER + childFolder))));
await()
.ignoreExceptions()
.until(
() ->
s3BlobFs.exists(
URI.create(
String.format(
FILE_FORMAT,
SCHEME,
bucket,
folder + DELIMITER + childFolder + DELIMITER + "a-ex.txt"))));

await()
.ignoreExceptions()
.until(
() ->
!s3BlobFs.exists(
URI.create(
String.format(
FILE_FORMAT,
SCHEME,
bucket,
folder + DELIMITER + childFolder + DELIMITER + "d-ex.txt"))));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.TimeoutException;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -169,6 +170,7 @@ public void testKafkaCanRestartOnError() {
}

@Test
@Disabled("Flaky test")
public void testDocumentInKafkaTransactionError() throws Exception {
KafkaConsumer kafkaConsumer = getTestKafkaConsumer();

Expand Down Expand Up @@ -205,6 +207,7 @@ public void testDocumentInKafkaTransactionError() throws Exception {
assertThat(responseObj.failedDocs()).isEqualTo(5);
assertThat(responseObj.errorMsg()).isNotNull();

// todo - this pretty consistently times out waiting to evaluate true
await()
.until(
() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.apache.commons.io.FileUtils;
import org.apache.lucene.index.IndexCommit;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Nested;
Expand All @@ -54,6 +55,7 @@

@SuppressWarnings("unused")
public class LuceneIndexStoreImplTest {

@BeforeAll
public static void beforeClass() {
Tracing.newBuilder().build();
Expand Down Expand Up @@ -397,9 +399,26 @@ private void testS3Snapshot(String bucket, String prefix) throws Exception {
assertThat(headObjectResponse.contentLength()).isEqualTo(fileToCopy.length());
}

// Download files from S3 to local FS.
String[] s3Files = copyFromS3(bucket, prefix, s3CrtBlobFs, tmpPath.toAbsolutePath());
assertThat(s3Files.length).isEqualTo(activeFiles.size());
// this try/retry/catch is to improve test reliability due to an AWS crt bug around mocked S3
// https://github.com/aws/aws-sdk-java-v2/issues/3658
await()
.ignoreExceptions()
.until(
() -> {
// clean the directory, in case a previous await try failed (would cause new copy to
// then fail)
FileUtils.cleanDirectory(tmpPath.toFile());
// Download files from S3 to local FS.
String[] s3Files =
copyFromS3(
bucket,
prefix,
s3CrtBlobFs,
tmpPath.toAbsolutePath()); // IO java.util.concurrent.ExecutionException:
// software.amazon.awssdk.core.exception.SdkClientException: Unexpected exception
// occurred: s3metaRequest is not initialized yet
return s3Files.length == activeFiles.size();
});

// Search files in local FS.
LogIndexSearcherImpl newSearcher =
Expand Down

0 comments on commit 56fdd33

Please sign in to comment.