Retry flaky unified tests

katcharov · katcharov · commit 6f3b56d6cd93 · 2024-11-21T14:10:05.000-07:00
JAVA-5393
diff --git a/driver-kotlin-coroutine/src/integration/kotlin/com/mongodb/kotlin/client/coroutine/UnifiedCrudTest.kt b/driver-kotlin-coroutine/src/integration/kotlin/com/mongodb/kotlin/client/coroutine/UnifiedCrudTest.kt
@@ -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)
         }
     }
 }
diff --git a/driver-kotlin-sync/src/integration/kotlin/com/mongodb/kotlin/client/UnifiedCrudTest.kt b/driver-kotlin-sync/src/integration/kotlin/com/mongodb/kotlin/client/UnifiedCrudTest.kt
@@ -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)
         }
     }
 }
diff --git a/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/ClientSideOperationTimeoutTest.java b/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/ClientSideOperationTimeoutTest.java
@@ -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,
diff --git a/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/UnifiedReactiveStreamsTest.java b/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/UnifiedReactiveStreamsTest.java
@@ -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;
@@ -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);
+    }
 }
diff --git a/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedSyncTest.java b/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedSyncTest.java
@@ -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() {
@@ -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);
+    }
 }
diff --git a/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java b/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java
@@ -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;
@@ -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;
@@ -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;
@@ -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;
@@ -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;
@@ -154,32 +162,51 @@ 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 forceFlaky = testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.FORCE_FLAKY);
+                boolean retry = forceFlaky || testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.RETRY);
+
+                int attempts = retry ? ATTEMPTS : 1;
+                if (forceFlaky) {
+                    attempts = 10;
+                }
+
+                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 * (forceFlaky ? -1 : 1),
+                            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;
     }
@@ -192,9 +219,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,
@@ -287,8 +317,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) {
     }
@@ -297,40 +328,58 @@ 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);
+        boolean forceFlaky = totalAttempts < 0;
+        if (!forceFlaky) {
+            assumeFalse(completed.contains(testName), "Skipping retryable test that succeeded");
+            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);
+            boolean lastAttempt = attemptNumber == Math.abs(totalAttempts);
+            if (forceFlaky || lastAttempt) {
+                throw e;
+            } else {
+                assumeFalse(completed.contains(testName), "Ignoring failure and retrying attempt " + attemptNumber);
             }
-            compareLogMessages(rootContext, definition, tweaks);
         }
     }
 
diff --git a/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestFailureValidator.java b/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestFailureValidator.java
@@ -36,19 +36,25 @@ final class UnifiedTestFailureValidator extends UnifiedSyncTest {
     @Override
     @BeforeEach
     public void setUp(
+            final String testName,
             @Nullable final String fileDescription,
             @Nullable final String testDescription,
             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.setUp(
+                    testName,
                     fileDescription,
                     testDescription,
                     directoryName,
+                    attemptNumber,
+                    totalAttempts,
                     schemaVersion,
                     runOnRequirements,
                     entitiesArray,
@@ -63,9 +69,12 @@ public void setUp(
     @ParameterizedTest
     @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,
@@ -74,9 +83,12 @@ public void shouldPassAllOutcomes(
         if (exception == null) {
             try {
                 super.shouldPassAllOutcomes(
+                        testName,
                         fileDescription,
                         testDescription,
                         directoryName,
+                        attemptNumber,
+                        totalAttempts,
                         schemaVersion,
                         runOnRequirements,
                         entitiesArray,
diff --git a/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java b/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java
diff --git a/driver-workload-executor/src/main/com/mongodb/workload/WorkloadExecutor.java b/driver-workload-executor/src/main/com/mongodb/workload/WorkloadExecutor.java

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ internal class UnifiedCrudTest() : UnifiedTest() {`
`24`	`24`	`@JvmStatic`
`25`	`25`	`@Throws(URISyntaxException::class, IOException::class)`
`26`	`26`	`fun data(): Collection<Arguments>? {`
`27`		`- return getTestData("unified-test-format/crud")`
	`27`	`+ return getTestData("unified-test-format/crud", true)`
`28`	`28`	`}`
`29`	`29`	`}`
`30`	`30`	`}`