Skip to content

Commit

Permalink
Assertion fixes, clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
@katcharov
katcharov committed Dec 5, 2024
1 parent 52d5a17 commit 3b2c915
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 25 deletions.
30 changes: 16 additions & 14 deletions driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
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;
Expand Down Expand Up @@ -96,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.abort;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
import static util.JsonPoweredTestHelper.getTestDocument;
Expand All @@ -107,7 +107,7 @@ public abstract class UnifiedTest {
"wait queue timeout errors include details about checked out connections");

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

@Nullable
private String fileDescription;
Expand Down Expand Up @@ -341,8 +341,9 @@ public void shouldPassAllOutcomes(
final BsonDocument definition) {
boolean forceFlaky = totalAttempts < 0;
if (!forceFlaky) {
assumeFalse(completed.contains(testName), "Skipping retryable test that succeeded");
completed.add(testName);
boolean ignoreThisTest = ignoreRemaining.contains(testName);
assumeFalse(ignoreThisTest, "Skipping a retryable test that already succeeded");
ignoreRemaining.add(testName);
}
try {
BsonArray operations = definition.getArray("operations");
Expand All @@ -369,20 +370,21 @@ 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
} catch (Throwable e) {
if (forceFlaky) {
throw e;
}

completed.remove(testName);
boolean lastAttempt = attemptNumber == Math.abs(totalAttempts);
if (forceFlaky || lastAttempt) {
if (!testDef.matchesThrowable(e)) {
// if the throwable is not matched, test definitions were not intended to apply; rethrow it
throw e;
}
boolean isLastAttempt = attemptNumber == Math.abs(totalAttempts);
if (isLastAttempt) {
throw e;
} else {
assumeFalse(completed.contains(testName), "Ignoring failure and retrying attempt " + attemptNumber);
}

ignoreRemaining.remove(testName);
abort("Ignoring failure and retrying attempt " + attemptNumber);
}
}

Expand Down
27 changes: 16 additions & 11 deletions driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ public static void applyCustomizations(final TestDef def) {
// TODO reasons for retry
// Exception in encryption library: ChangeCipherSpec message sequence violation
def.retry("TODO reason")
.whenExceptionContains("ChangeCipherSpec message sequence violation")
.whenFailureContains("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")
.whenFailureContains("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")
Expand Down Expand Up @@ -295,7 +295,7 @@ public static final class TestDef {
private final boolean reactive;

private final List<Modifier> modifiers = new ArrayList<>();
private Function<AssertionFailedError, Boolean> matchesError;
private Function<Throwable, Boolean> matchesThrowable;

private TestDef(final String dir, final String file, final String test, final boolean reactive) {
this.dir = dir;
Expand Down Expand Up @@ -379,9 +379,9 @@ public boolean wasAssignedModifier(final Modifier modifier) {
return this.modifiers.contains(modifier);
}

public boolean matchesError(final AssertionFailedError e) {
if (matchesError != null) {
return matchesError.apply(e);
public boolean matchesThrowable(final Throwable e) {
if (matchesThrowable != null) {
return matchesThrowable.apply(e);
}
return false;
}
Expand All @@ -396,7 +396,7 @@ public static final class TestApplicator {
private boolean matchWasPerformed = false;

private final List<Modifier> modifiersToApply;
private Function<AssertionFailedError, Boolean> matchesError;
private Function<Throwable, Boolean> matchesThrowable;

private TestApplicator(
final TestDef testDef,
Expand All @@ -416,7 +416,7 @@ private TestApplicator onMatch(final boolean match) {
}
if (match) {
this.testDef.modifiers.addAll(this.modifiersToApply);
this.testDef.matchesError = this.matchesError;
this.testDef.matchesThrowable = this.matchesThrowable;
}
return this;
}
Expand Down Expand Up @@ -509,9 +509,14 @@ public TestApplicator when(final Supplier<Boolean> precondition) {
return this;
}

public TestApplicator whenExceptionContains(final String fragment) {
this.matchesError = (final AssertionFailedError e) -> {
return e.getCause().getMessage().contains(fragment);
public TestApplicator whenFailureContains(final String messageFragment) {
this.matchesThrowable = (final Throwable e) -> {
// inspect the cause for failed assertions with a cause
if (e instanceof AssertionFailedError && e.getCause() != null) {
return e.getCause().getMessage().contains(messageFragment);
} else {
return e.getMessage().contains(messageFragment);
}
};
return this;
}
Expand Down

0 comments on commit 3b2c915

Please sign in to comment.