From 52d5a1764cf6554cb896ed8346ec4bbcb44324ed Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Wed, 4 Dec 2024 17:01:51 -0700 Subject: [PATCH] Add whenExceptionContains, fixes --- .../mongodb/client/unified/UnifiedTest.java | 19 ++++--- .../unified/UnifiedTestModifications.java | 54 +++++++++++++++---- 2 files changed, 57 insertions(+), 16 deletions(-) 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 index ff703b7bdc..2057c0e8d0 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java @@ -84,7 +84,8 @@ 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.Modifier; +import static com.mongodb.client.unified.UnifiedTestModifications.applyCustomizations; import static com.mongodb.client.unified.UnifiedTestModifications.testDef; import static java.util.Collections.singletonList; import static java.util.stream.Collectors.toList; @@ -173,10 +174,10 @@ protected static Collection getTestData(final String directory, final String testDescription = testDocument.getString("description").getValue(); String fileDescription = fileDocument.getString("description").getValue(); TestDef testDef = testDef(directory, fileDescription, testDescription, isReactive); - doSkips(testDef); + applyCustomizations(testDef); - boolean forceFlaky = testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.FORCE_FLAKY); - boolean retry = forceFlaky || testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.RETRY); + boolean forceFlaky = testDef.wasAssignedModifier(Modifier.FORCE_FLAKY); + boolean retry = forceFlaky || testDef.wasAssignedModifier(Modifier.RETRY); int attempts = retry ? ATTEMPTS : 1; if (forceFlaky) { @@ -241,9 +242,9 @@ public void setUp( rootContext.getAssertionContext().push(ContextElement.ofTest(definition)); ignoreExtraEvents = false; testDef = testDef(directoryName, fileDescription, testDescription, isReactive()); - UnifiedTestModifications.doSkips(testDef); + applyCustomizations(testDef); - boolean skip = testDef.wasAssignedModifier(UnifiedTestModifications.Modifier.SKIP); + boolean skip = testDef.wasAssignedModifier(Modifier.SKIP); assumeFalse(skip, "Skipping test"); skips(fileDescription, testDescription); @@ -369,6 +370,12 @@ public void shouldPassAllOutcomes( compareLogMessages(rootContext, definition, tweaks); } } catch (AssertionFailedError e) { + assertTrue(testDef.wasAssignedModifier(Modifier.RETRY)); + if (!testDef.matchesError(e)) { + // if the error is not matched, test definitions were not intended to apply; throw it + throw e; + } + completed.remove(testName); boolean lastAttempt = attemptNumber == Math.abs(totalAttempts); if (forceFlaky || lastAttempt) { 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 index 4da6f9308b..58ce02dfa6 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java +++ b/driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java @@ -17,10 +17,12 @@ package com.mongodb.client.unified; import com.mongodb.assertions.Assertions; +import org.opentest4j.AssertionFailedError; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.Function; import java.util.function.Supplier; import static com.mongodb.ClusterFixture.isDataLakeTest; @@ -28,23 +30,28 @@ import static com.mongodb.ClusterFixture.isServerlessTest; import static com.mongodb.ClusterFixture.isSharded; import static com.mongodb.ClusterFixture.serverVersionLessThan; -import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.FORCE_FLAKY; import static com.mongodb.assertions.Assertions.assertNotNull; import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.IGNORE_EXTRA_EVENTS; +import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.RETRY; import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.SKIP; import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.SLEEP_AFTER_CURSOR_CLOSE; import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.SLEEP_AFTER_CURSOR_OPEN; import static com.mongodb.client.unified.UnifiedTestModifications.Modifier.WAIT_FOR_BATCH_CURSOR_CREATION; public final class UnifiedTestModifications { - public static void doSkips(final TestDef def) { + public static void applyCustomizations(final TestDef def) { // TODO reasons for retry - def.modify(FORCE_FLAKY) - // Exception in encryption library: ChangeCipherSpec message sequence violation - .test("client-side-encryption", "namedKMS-createDataKey", "create datakey with named KMIP KMS provider") - // Number of checked out connections must match expected - .test("load-balancers", "cursors are correctly pinned to connections for load-balanced clusters", "pinned connections are returned after a network error during a killCursors request") + // Exception in encryption library: ChangeCipherSpec message sequence violation + def.retry("TODO reason") + .whenExceptionContains("ChangeCipherSpec message sequence violation") + .test("client-side-encryption", "namedKMS-createDataKey", "create datakey with named KMIP KMS provider"); + + def.retry("TODO reason") + .whenExceptionContains("Number of checked out connections must match expected") + .test("load-balancers", "cursors are correctly pinned to connections for load-balanced clusters", "pinned connections are returned after a network error during a killCursors request"); + + def.retry("TODO reason") .test("client-side-encryption", "namedKMS-rewrapManyDataKey", "rewrap to kmip:name1"); // atlas-data-lake @@ -288,6 +295,7 @@ public static final class TestDef { private final boolean reactive; private final List modifiers = new ArrayList<>(); + private Function matchesError; private TestDef(final String dir, final String file, final String test, final boolean reactive) { this.dir = dir; @@ -332,7 +340,8 @@ public TestApplicator skipNoncompliant(final String reason) { * @param reason reason for skipping the test */ public TestApplicator skipNoncompliantReactive(final String reason) { - return new TestApplicator(this, reason, SKIP); + return new TestApplicator(this, reason, SKIP) + .when(() -> isReactive()); } /** @@ -350,6 +359,14 @@ public TestApplicator skipUnknownReason(final String reason) { return new TestApplicator(this, reason, SKIP); } + + /** + * The test will be retried, for the reason provided + */ + public TestApplicator retry(final String reason) { + return new TestApplicator(this, reason, RETRY); + } + public TestApplicator modify(final Modifier... modifiers) { return new TestApplicator(this, null, modifiers); } @@ -361,6 +378,13 @@ public boolean isReactive() { public boolean wasAssignedModifier(final Modifier modifier) { return this.modifiers.contains(modifier); } + + public boolean matchesError(final AssertionFailedError e) { + if (matchesError != null) { + return matchesError.apply(e); + } + return false; + } } /** @@ -368,17 +392,19 @@ public boolean wasAssignedModifier(final Modifier modifier) { */ public static final class TestApplicator { private final TestDef testDef; - private final List modifiersToApply; private Supplier precondition; private boolean matchWasPerformed = false; + private final List modifiersToApply; + private Function matchesError; + private TestApplicator( final TestDef testDef, final String reason, final Modifier... modifiersToApply) { this.testDef = testDef; this.modifiersToApply = Arrays.asList(modifiersToApply); - if (this.modifiersToApply.contains(SKIP)) { + if (this.modifiersToApply.contains(SKIP) || this.modifiersToApply.contains(RETRY)) { assertNotNull(reason); } } @@ -390,6 +416,7 @@ private TestApplicator onMatch(final boolean match) { } if (match) { this.testDef.modifiers.addAll(this.modifiersToApply); + this.testDef.matchesError = this.matchesError; } return this; } @@ -481,6 +508,13 @@ public TestApplicator when(final Supplier precondition) { this.precondition = precondition; return this; } + + public TestApplicator whenExceptionContains(final String fragment) { + this.matchesError = (final AssertionFailedError e) -> { + return e.getCause().getMessage().contains(fragment); + }; + return this; + } } public enum Modifier {