Skip to content

Commit

Permalink
Retry flaky unified tests
Browse files Browse the repository at this point in the history
  • Loading branch information
katcharov committed Nov 21, 2024
1 parent 90976e4 commit 8e8d7a3
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class UnifiedCrudTest() : UnifiedTest() {
@JvmStatic
@Throws(URISyntaxException::class, IOException::class)
fun data(): Collection<Arguments>? {
return getTestData("unified-test-format/crud")
return getTestData("unified-test-format/crud", true)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class UnifiedCrudTest() : UnifiedTest() {
@JvmStatic
@Throws(URISyntaxException::class, IOException::class)
fun data(): Collection<Arguments>? {
return getTestData("unified-test-format/crud")
return getTestData("unified-test-format/crud", false)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,25 @@ The Reactive Streams specification prevents us from allowing a subsequent next c
@MethodSource("data")
@Override
public void shouldPassAllOutcomes(
final String testName,
@Nullable final String fileDescription,
@Nullable final String testDescription,
@Nullable final String directoryName,
final int attemptNumber,
final int totalAttempts,
final String schemaVersion,
@Nullable final BsonArray runOnRequirements,
final BsonArray entitiesArray,
final BsonArray initialData,
final BsonDocument definition) {
try {
super.shouldPassAllOutcomes(fileDescription,
super.shouldPassAllOutcomes(
testName,
fileDescription,
testDescription,
directoryName,
attemptNumber,
totalAttempts,
schemaVersion,
runOnRequirements,
entitiesArray,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@
import com.mongodb.client.unified.UnifiedTest;
import com.mongodb.client.unified.UnifiedTestModifications;
import com.mongodb.client.vault.ClientEncryption;
import com.mongodb.lang.NonNull;
import com.mongodb.reactivestreams.client.MongoClients;
import com.mongodb.reactivestreams.client.gridfs.GridFSBuckets;
import com.mongodb.reactivestreams.client.internal.vault.ClientEncryptionImpl;
import com.mongodb.reactivestreams.client.syncadapter.SyncClientEncryption;
import com.mongodb.reactivestreams.client.syncadapter.SyncGridFSBucket;
import com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient;
import com.mongodb.reactivestreams.client.syncadapter.SyncMongoDatabase;
import org.junit.jupiter.params.provider.Arguments;

import java.io.IOException;
import java.net.URISyntaxException;
import java.util.Collection;

import static com.mongodb.client.unified.UnifiedTestModifications.Modifier;
import static com.mongodb.client.unified.UnifiedTestModifications.TestDef;
Expand Down Expand Up @@ -94,4 +100,9 @@ protected void postCleanUp(final TestDef testDef) {
disableSleep();
}
}

@NonNull
protected static Collection<Arguments> getTestData(final String directory) throws URISyntaxException, IOException {
return getTestData(directory, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
import com.mongodb.client.gridfs.GridFSBuckets;
import com.mongodb.client.internal.ClientEncryptionImpl;
import com.mongodb.client.vault.ClientEncryption;
import com.mongodb.lang.NonNull;
import org.junit.jupiter.params.provider.Arguments;

import java.io.IOException;
import java.net.URISyntaxException;
import java.util.Collection;

public abstract class UnifiedSyncTest extends UnifiedTest {
protected UnifiedSyncTest() {
Expand All @@ -44,4 +50,9 @@ protected GridFSBucket createGridFSBucket(final MongoDatabase database) {
protected ClientEncryption createClientEncryption(final MongoClient keyVaultClient, final ClientEncryptionSettings clientEncryptionSettings) {
return new ClientEncryptionImpl(keyVaultClient, clientEncryptionSettings);
}

@NonNull
protected static Collection<Arguments> getTestData(final String directory) throws URISyntaxException, IOException {
return getTestData(directory, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,27 @@
import com.mongodb.MongoNamespace;
import com.mongodb.ReadPreference;
import com.mongodb.UnixServerAddress;
import com.mongodb.client.unified.UnifiedTestModifications.TestDef;
import com.mongodb.event.TestServerMonitorListener;
import com.mongodb.internal.logging.LogMessage;
import com.mongodb.logging.TestLoggingInterceptor;
import com.mongodb.WriteConcern;
import com.mongodb.client.ClientSession;
import com.mongodb.client.MongoClient;
import com.mongodb.client.MongoDatabase;
import com.mongodb.client.gridfs.GridFSBucket;
import com.mongodb.client.model.Filters;
import com.mongodb.client.test.CollectionHelper;
import com.mongodb.client.unified.UnifiedTestModifications.TestDef;
import com.mongodb.client.vault.ClientEncryption;
import com.mongodb.connection.ClusterDescription;
import com.mongodb.connection.ClusterType;
import com.mongodb.connection.ServerDescription;
import com.mongodb.event.CommandEvent;
import com.mongodb.event.CommandStartedEvent;
import com.mongodb.event.TestServerMonitorListener;
import com.mongodb.internal.connection.TestCommandListener;
import com.mongodb.internal.connection.TestConnectionPoolListener;
import com.mongodb.internal.logging.LogMessage;
import com.mongodb.lang.NonNull;
import com.mongodb.lang.Nullable;
import com.mongodb.logging.TestLoggingInterceptor;
import com.mongodb.test.AfterBeforeParameterResolver;
import org.bson.BsonArray;
import org.bson.BsonBoolean;
Expand All @@ -57,14 +57,17 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.opentest4j.AssertionFailedError;
import org.opentest4j.TestAbortedException;

import java.io.File;
import java.io.IOException;
import java.net.URISyntaxException;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ExecutionException;
Expand All @@ -81,6 +84,7 @@
import static com.mongodb.client.test.CollectionHelper.getCurrentClusterTime;
import static com.mongodb.client.test.CollectionHelper.killAllSessions;
import static com.mongodb.client.unified.RunOnRequirementsMatcher.runOnRequirementsMet;
import static com.mongodb.client.unified.UnifiedTestModifications.doSkips;
import static com.mongodb.client.unified.UnifiedTestModifications.testDef;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toList;
Expand All @@ -91,6 +95,7 @@
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
import static util.JsonPoweredTestHelper.getTestDocument;
import static util.JsonPoweredTestHelper.getTestFiles;
Expand All @@ -100,6 +105,9 @@ public abstract class UnifiedTest {
private static final Set<String> PRESTART_POOL_ASYNC_WORK_MANAGER_FILE_DESCRIPTIONS = Collections.singleton(
"wait queue timeout errors include details about checked out connections");

public static final int ATTEMPTS = 3;
private static Set<String> completed = new HashSet<>();

@Nullable
private String fileDescription;
private String schemaVersion;
Expand Down Expand Up @@ -154,32 +162,47 @@ public Entities getEntities() {
}

@NonNull
protected static Collection<Arguments> getTestData(final String directory) throws URISyntaxException, IOException {
protected static Collection<Arguments> getTestData(final String directory, final boolean isReactive)
throws URISyntaxException, IOException {
List<Arguments> data = new ArrayList<>();
for (File file : getTestFiles("/" + directory + "/")) {
BsonDocument fileDocument = getTestDocument(file);

for (BsonValue cur : fileDocument.getArray("tests")) {
data.add(UnifiedTest.createTestData(directory, fileDocument, cur.asDocument()));

final BsonDocument testDocument = cur.asDocument();
String testDescription = testDocument.getString("description").getValue();
String fileDescription = fileDocument.getString("description").getValue();
TestDef testDef = testDef(directory, fileDescription, testDescription, isReactive);
doSkips(testDef);

boolean retry = testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.RETRY);
int attempts = retry ? ATTEMPTS : 1;


for (int attempt = 1; attempt <= attempts; attempt++) {
String testName = !retry
? MessageFormat.format("{0}: {1}", fileDescription, testDescription)
: MessageFormat.format(
"{0}: {1} ({2} of {3})",
fileDescription, testDescription, attempt, attempts);
data.add(Arguments.of(
testName,
fileDescription,
testDescription,
directory,
attempt,
attempts,
fileDocument.getString("schemaVersion").getValue(),
fileDocument.getArray("runOnRequirements", null),
fileDocument.getArray("createEntities", new BsonArray()),
fileDocument.getArray("initialData", new BsonArray()),
testDocument));
}
}
}
return data;
}

@NonNull
private static Arguments createTestData(
final String directory, final BsonDocument fileDocument, final BsonDocument testDocument) {
return Arguments.of(
fileDocument.getString("description").getValue(),
testDocument.getString("description").getValue(),
directory,
fileDocument.getString("schemaVersion").getValue(),
fileDocument.getArray("runOnRequirements", null),
fileDocument.getArray("createEntities", new BsonArray()),
fileDocument.getArray("initialData", new BsonArray()),
testDocument);
}

protected BsonDocument getDefinition() {
return definition;
}
Expand All @@ -192,9 +215,12 @@ protected BsonDocument getDefinition() {

@BeforeEach
public void setUp(
final String testName,
@Nullable final String fileDescription,
@Nullable final String testDescription,
@Nullable final String directoryName,
final int attemptNumber,
final int totalAttempts,
final String schemaVersion,
@Nullable final BsonArray runOnRequirements,
final BsonArray entitiesArray,
Expand All @@ -216,7 +242,11 @@ public void setUp(
ignoreExtraEvents = false;
testDef = testDef(directoryName, fileDescription, testDescription, isReactive());
UnifiedTestModifications.doSkips(testDef);

boolean skip = testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.SKIP);
assumeFalse(skip, "Skipping test");
skips(fileDescription, testDescription);

assertTrue(
schemaVersion.equals("1.0")
|| schemaVersion.equals("1.1")
Expand Down Expand Up @@ -283,8 +313,9 @@ protected void postCleanUp(final TestDef testDef) {
}

/**
* This method is called once per {@link #setUp(String, String, String, String, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonDocument)},
* unless {@link #setUp(String, String, String, String, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonDocument)} fails unexpectedly.
* This method is called once per
* {@link #setUp(String, String, String, String, int, int, String, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonDocument)}, unless
* {@link #setUp(String, String, String, String, int, int, String, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonArray, org.bson.BsonDocument)} fails unexpectedly.
*/
protected void skips(final String fileDescription, final String testDescription) {
}
Expand All @@ -293,40 +324,54 @@ protected boolean isReactive() {
return false;
}

@ParameterizedTest(name = "{0}: {1}")
@ParameterizedTest(name = "{0}")
@MethodSource("data")
public void shouldPassAllOutcomes(
final String testName,
@Nullable final String fileDescription,
@Nullable final String testDescription,
@Nullable final String directoryName,
final int attemptNumber,
final int totalAttempts,
final String schemaVersion,
@Nullable final BsonArray runOnRequirements,
final BsonArray entitiesArray,
final BsonArray initialData,
final BsonDocument definition) {
BsonArray operations = definition.getArray("operations");
for (int i = 0; i < operations.size(); i++) {
BsonValue cur = operations.get(i);
assertOperation(rootContext, cur.asDocument(), i);
}
assumeFalse(completed.contains(testName), "Skipping test already performed");
completed.add(testName);
try {
BsonArray operations = definition.getArray("operations");
for (int i = 0; i < operations.size(); i++) {
BsonValue cur = operations.get(i);
assertOperation(rootContext, cur.asDocument(), i);
}

if (definition.containsKey("outcome")) {
assertOutcome(rootContext);
}
if (definition.containsKey("outcome")) {
assertOutcome(rootContext);
}

if (definition.containsKey("expectEvents")) {
compareEvents(rootContext, definition);
}
if (definition.containsKey("expectEvents")) {
compareEvents(rootContext, definition);
}

if (definition.containsKey("expectLogMessages")) {
ArrayList<LogMatcher.Tweak> tweaks = new ArrayList<>(singletonList(
// `LogMessage.Entry.Name.OPERATION` is not supported, therefore we skip matching its value
LogMatcher.Tweak.skip(LogMessage.Entry.Name.OPERATION)));
if (getMongoClientSettings().getClusterSettings()
.getHosts().stream().anyMatch(serverAddress -> serverAddress instanceof UnixServerAddress)) {
tweaks.add(LogMatcher.Tweak.skip(LogMessage.Entry.Name.SERVER_PORT));
if (definition.containsKey("expectLogMessages")) {
ArrayList<LogMatcher.Tweak> tweaks = new ArrayList<>(singletonList(
// `LogMessage.Entry.Name.OPERATION` is not supported, therefore we skip matching its value
LogMatcher.Tweak.skip(LogMessage.Entry.Name.OPERATION)));
if (getMongoClientSettings().getClusterSettings()
.getHosts().stream().anyMatch(serverAddress -> serverAddress instanceof UnixServerAddress)) {
tweaks.add(LogMatcher.Tweak.skip(LogMessage.Entry.Name.SERVER_PORT));
}
compareLogMessages(rootContext, definition, tweaks);
}
} catch (AssertionFailedError e) {
completed.remove(testName);
if (attemptNumber == totalAttempts) { // last attempt
throw e;
} else {
assumeFalse(completed.contains(testName), "Ignoring failure and retrying attempt " + attemptNumber);
}
compareLogMessages(rootContext, definition, tweaks);
}
}

Expand Down
Loading

0 comments on commit 8e8d7a3

Please sign in to comment.