From 290842f6061b1f6848becb1ec05447c466fb0300 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Thu, 21 Nov 2024 13:17:50 -0700 Subject: [PATCH 1/3] Clean up TestDef API, clarify naming --- .../mongodb/client/unified/UnifiedTest.java | 5 ++ .../unified/UnifiedTestFailureValidator.java | 4 +- .../unified/UnifiedTestModifications.java | 64 ++++++++----------- 3 files changed, 34 insertions(+), 39 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 d1d1fc58c8a..d55feb55395 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 @@ -91,6 +91,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; @@ -216,7 +217,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") 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 index b5bf53f4b88..88458f8af8e 100644 --- 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,9 +36,9 @@ final class UnifiedTestFailureValidator extends UnifiedSyncTest { @Override @BeforeEach public void setUp( - final String directoryName, @Nullable final String fileDescription, @Nullable final String testDescription, + final String directoryName, final String schemaVersion, @Nullable final BsonArray runOnRequirements, final BsonArray entitiesArray, @@ -46,9 +46,9 @@ public void setUp( final BsonDocument definition) { try { super.setUp( - directoryName, fileDescription, testDescription, + directoryName, 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 index ea8180ead0e..dd03d3dc2e1 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 @@ -18,7 +18,6 @@ import com.mongodb.assertions.Assertions; import com.mongodb.lang.NonNull; -import com.mongodb.lang.Nullable; import java.util.ArrayList; import java.util.Arrays; @@ -34,7 +33,6 @@ 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; -import static org.junit.jupiter.api.Assumptions.assumeFalse; public final class UnifiedTestModifications { public static void doSkips(final TestDef def) { @@ -90,7 +88,7 @@ public static void doSkips(final TestDef def) { // added as part of https://jira.mongodb.org/browse/JAVA-4976 , but unknown Jira to complete // The implementation of the functionality related to clearing the connection pool before closing the connection // will be carried out once the specification is finalized and ready. - def.skipTodo("") + def.skipUnknownReason("") .test("connection-monitoring-and-pooling/logging", "connection-logging", "Connection checkout fails due to error establishing connection"); // load-balancers @@ -129,7 +127,7 @@ public static void doSkips(final TestDef def) { .test("crud", "count", "Deprecated count without a filter") .test("crud", "count", "Deprecated count with a filter") .test("crud", "count", "Deprecated count with skip and limit"); - def.skipTodo("See downstream changes comment on https://jira.mongodb.org/browse/JAVA-4275") + def.skipUnknownReason("See downstream changes comment on https://jira.mongodb.org/browse/JAVA-4275") .test("crud", "findOneAndReplace-hint-unacknowledged", "Unacknowledged findOneAndReplace with hint string on 4.4+ server") .test("crud", "findOneAndReplace-hint-unacknowledged", "Unacknowledged findOneAndReplace with hint document on 4.4+ server") .test("crud", "findOneAndUpdate-hint-unacknowledged", "Unacknowledged findOneAndUpdate with hint string on 4.4+ server") @@ -292,54 +290,54 @@ private TestDef(final String dir, final String file, final String test, final bo * Test is skipped because it is pending implementation, and there is * a Jira ticket tracking this which has more information. * - * @param skip reason for skipping the test; must start with a Jira URL + * @param ticket reason for skipping the test; must start with a Jira URL */ - public TestApplicator skipJira(final String skip) { - Assertions.assertTrue(skip.startsWith("https://jira.mongodb.org/browse/JAVA-")); - return new TestApplicator(this, skip); + public TestApplicator skipJira(final String ticket) { + Assertions.assertTrue(ticket.startsWith("https://jira.mongodb.org/browse/JAVA-")); + return new TestApplicator(this, ticket); } /** * Test is skipped because the feature under test was deprecated, and * was removed in the Java driver. * - * @param skip reason for skipping the test + * @param reason reason for skipping the test */ - public TestApplicator skipDeprecated(final String skip) { - return new TestApplicator(this, skip); + public TestApplicator skipDeprecated(final String reason) { + return new TestApplicator(this, reason); } /** * Test is skipped because the Java driver cannot comply with the spec. * - * @param skip reason for skipping the test + * @param reason reason for skipping the test */ - public TestApplicator skipNoncompliant(final String skip) { - return new TestApplicator(this, skip); + public TestApplicator skipNoncompliant(final String reason) { + return new TestApplicator(this, reason); } /** * Test is skipped because the Java Reactive driver cannot comply with the spec. * - * @param skip reason for skipping the test + * @param reason reason for skipping the test */ - public TestApplicator skipNoncompliantReactive(final String skip) { - return new TestApplicator(this, skip); + public TestApplicator skipNoncompliantReactive(final String reason) { + return new TestApplicator(this, reason); } /** * The test is skipped, as specified. This should be paired with a * "when" clause. */ - public TestApplicator skipAccordingToSpec(final String skip) { - return new TestApplicator(this, skip); + public TestApplicator skipAccordingToSpec(final String reason) { + return new TestApplicator(this, reason); } /** * The test is skipped for an unknown reason. */ - public TestApplicator skipTodo(final String skip) { - return new TestApplicator(this, skip); + public TestApplicator skipUnknownReason(final String reason) { + return new TestApplicator(this, reason); } public TestApplicator modify(final Modifier... modifiers) { @@ -360,10 +358,6 @@ public boolean wasAssignedModifier(final Modifier modifier) { */ public static final class TestApplicator { private final TestDef testDef; - - private final boolean shouldSkip; - @Nullable - private final String reasonToApply; private final List modifiersToApply; private Supplier precondition; private boolean matchWasPerformed = false; @@ -372,19 +366,15 @@ private TestApplicator( final TestDef testDef, final List modifiersToApply) { this.testDef = testDef; - this.shouldSkip = false; - this.reasonToApply = null; this.modifiersToApply = modifiersToApply; } private TestApplicator( final TestDef testDef, @NonNull - final String reason) { + final String skipReason) { this.testDef = testDef; - this.shouldSkip = true; - this.reasonToApply = reason; - this.modifiersToApply = new ArrayList<>(); + this.modifiersToApply = Arrays.asList(Modifier.SKIP); } private TestApplicator onMatch(final boolean match) { @@ -392,12 +382,8 @@ private TestApplicator onMatch(final boolean match) { if (precondition != null && !precondition.get()) { return this; } - if (shouldSkip) { - assumeFalse(match, reasonToApply); - } else { - if (match) { - this.testDef.modifiers.addAll(this.modifiersToApply); - } + if (match) { + this.testDef.modifiers.addAll(this.modifiersToApply); } return this; } @@ -510,5 +496,9 @@ public enum Modifier { * Reactive only. */ WAIT_FOR_BATCH_CURSOR_CREATION, + /** + * Skip the test. + */ + SKIP, } } From ce373067a9747ec1928a9710f7610fa2ea98cfa5 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Mon, 25 Nov 2024 09:28:22 -0700 Subject: [PATCH 2/3] Update driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java Co-authored-by: Nabil Hachicha --- .../com/mongodb/client/unified/UnifiedTestModifications.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dd03d3dc2e1..a8dcb30da81 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 @@ -374,7 +374,7 @@ private TestApplicator( @NonNull final String skipReason) { this.testDef = testDef; - this.modifiersToApply = Arrays.asList(Modifier.SKIP); + this.modifiersToApply = Collections.singletonList(Modifier.SKIP); } private TestApplicator onMatch(final boolean match) { From 041e0105faee6665b77a8682e816c87824145922 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Mon, 25 Nov 2024 10:39:51 -0700 Subject: [PATCH 3/3] PR fixes --- .../unified/UnifiedTestModifications.java | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) 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 a8dcb30da81..5b11218518f 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,7 +17,6 @@ package com.mongodb.client.unified; import com.mongodb.assertions.Assertions; -import com.mongodb.lang.NonNull; import java.util.ArrayList; import java.util.Arrays; @@ -29,7 +28,9 @@ import static com.mongodb.ClusterFixture.isServerlessTest; import static com.mongodb.ClusterFixture.isSharded; import static com.mongodb.ClusterFixture.serverVersionLessThan; +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.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; @@ -294,7 +295,7 @@ private TestDef(final String dir, final String file, final String test, final bo */ public TestApplicator skipJira(final String ticket) { Assertions.assertTrue(ticket.startsWith("https://jira.mongodb.org/browse/JAVA-")); - return new TestApplicator(this, ticket); + return new TestApplicator(this, ticket, SKIP); } /** @@ -304,7 +305,7 @@ public TestApplicator skipJira(final String ticket) { * @param reason reason for skipping the test */ public TestApplicator skipDeprecated(final String reason) { - return new TestApplicator(this, reason); + return new TestApplicator(this, reason, SKIP); } /** @@ -313,7 +314,7 @@ public TestApplicator skipDeprecated(final String reason) { * @param reason reason for skipping the test */ public TestApplicator skipNoncompliant(final String reason) { - return new TestApplicator(this, reason); + return new TestApplicator(this, reason, SKIP); } /** @@ -322,7 +323,7 @@ public TestApplicator skipNoncompliant(final String reason) { * @param reason reason for skipping the test */ public TestApplicator skipNoncompliantReactive(final String reason) { - return new TestApplicator(this, reason); + return new TestApplicator(this, reason, SKIP); } /** @@ -330,18 +331,18 @@ public TestApplicator skipNoncompliantReactive(final String reason) { * "when" clause. */ public TestApplicator skipAccordingToSpec(final String reason) { - return new TestApplicator(this, reason); + return new TestApplicator(this, reason, SKIP); } /** * The test is skipped for an unknown reason. */ public TestApplicator skipUnknownReason(final String reason) { - return new TestApplicator(this, reason); + return new TestApplicator(this, reason, SKIP); } public TestApplicator modify(final Modifier... modifiers) { - return new TestApplicator(this, Arrays.asList(modifiers)); + return new TestApplicator(this, null, modifiers); } public boolean isReactive() { @@ -364,17 +365,13 @@ public static final class TestApplicator { private TestApplicator( final TestDef testDef, - final List modifiersToApply) { + final String reason, + final Modifier... modifiersToApply) { this.testDef = testDef; - this.modifiersToApply = modifiersToApply; - } - - private TestApplicator( - final TestDef testDef, - @NonNull - final String skipReason) { - this.testDef = testDef; - this.modifiersToApply = Collections.singletonList(Modifier.SKIP); + this.modifiersToApply = Arrays.asList(modifiersToApply); + if (this.modifiersToApply.contains(SKIP)) { + assertNotNull(reason); + } } private TestApplicator onMatch(final boolean match) {