Clean up unified tests via skipping API (#1551)

Clean up unified tests via skipping API

Author: katcharov

driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/UnifiedRetryableReadsTest.java

@@ -28,13 +28,14 @@
 import java.util.Collection;
 
 import static com.mongodb.client.unified.UnifiedRetryableReadsTest.doSkips;
+import static com.mongodb.client.unified.UnifiedTestSkips.testDef;
 import static com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient.disableWaitForBatchCursorCreation;
 import static com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient.enableWaitForBatchCursorCreation;
 
 final class UnifiedRetryableReadsTest extends UnifiedReactiveStreamsTest {
     @Override
     protected void skips(final String fileDescription, final String testDescription) {
-        doSkips(fileDescription, testDescription);
+        doSkips(testDef("unified-test-format/retryable-reads", fileDescription, testDescription));
     }
 
     @Override

driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/UnifiedRetryableWritesTest.java

@@ -23,11 +23,12 @@
 import java.util.Collection;
 
 import static com.mongodb.client.unified.UnifiedRetryableWritesTest.doSkips;
+import static com.mongodb.client.unified.UnifiedTestSkips.testDef;
 
 final class UnifiedRetryableWritesTest extends UnifiedReactiveStreamsTest {
     @Override
     protected void skips(final String fileDescription, final String testDescription) {
-        doSkips(testDescription);
+        doSkips(testDef("unified-test-format/retryable-writes", fileDescription, testDescription));
     }
 
     private static Collection<Arguments> data() throws URISyntaxException, IOException {

driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/unified/UnifiedServerDiscoveryAndMonitoringTest.java

@@ -22,13 +22,16 @@
 import java.net.URISyntaxException;
 import java.util.Collection;
 
+import static com.mongodb.client.unified.UnifiedTestSkips.testDef;
+
 final class UnifiedServerDiscoveryAndMonitoringTest extends UnifiedReactiveStreamsTest {
     private static Collection<Arguments> data() throws URISyntaxException, IOException {
         return getTestData("unified-test-format/server-discovery-and-monitoring");
     }
 
     @Override
     protected void skips(final String fileDescription, final String testDescription) {
-        com.mongodb.client.unified.UnifiedServerDiscoveryAndMonitoringTest.doSkips(fileDescription, testDescription);
+        com.mongodb.client.unified.UnifiedServerDiscoveryAndMonitoringTest.doSkips(
+                testDef("unified-test-format/server-discovery-and-monitoring", fileDescription, testDescription));
     }
 }

driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedRetryableReadsTest.java

@@ -22,28 +22,28 @@
 import java.net.URISyntaxException;
 import java.util.Collection;
 
-import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static com.mongodb.client.unified.UnifiedTestSkips.TestDef;
+import static com.mongodb.client.unified.UnifiedTestSkips.testDef;
 
 public final class UnifiedRetryableReadsTest extends UnifiedSyncTest {
     @Override
     protected void skips(final String fileDescription, final String testDescription) {
-        doSkips(fileDescription, testDescription);
+        doSkips(testDef("unified-test-format/retryable-reads", fileDescription, testDescription));
     }
 
-    public static void doSkips(final String fileDescription, @SuppressWarnings("unused") final String testDescription) {
-        // Skipped because driver removed the deprecated count methods
-        assumeFalse(fileDescription.equals("count"));
-        assumeFalse(fileDescription.equals("count-serverErrors"));
-        // Skipped because the driver never had these methods
-        assumeFalse(fileDescription.equals("listDatabaseObjects"));
-        assumeFalse(fileDescription.equals("listDatabaseObjects-serverErrors"));
-        assumeFalse(fileDescription.equals("listCollectionObjects"));
-        assumeFalse(fileDescription.equals("listCollectionObjects-serverErrors"));
-
-        // JAVA-5224
-        assumeFalse(fileDescription.equals("ReadConcernMajorityNotAvailableYet is a retryable read")
-                && testDescription.equals("Find succeeds on second attempt after ReadConcernMajorityNotAvailableYet"),
-                "JAVA-5224");
+    public static void doSkips(final TestDef def) {
+        def.skipDeprecated("Deprecated feature removed")
+                .file("retryable-reads", "count")
+                .file("retryable-reads", "count-serverErrors");
+
+        def.skipDeprecated("Deprecated feature never implemented")
+                .file("retryable-reads", "listDatabaseObjects")
+                .file("retryable-reads", "listDatabaseObjects-serverErrors")
+                .file("retryable-reads", "listCollectionObjects")
+                .file("retryable-reads", "listCollectionObjects-serverErrors");
+
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-5224")
+                .test("retryable-reads", "ReadConcernMajorityNotAvailableYet is a retryable read", "Find succeeds on second attempt after ReadConcernMajorityNotAvailableYet");
     }
 
     private static Collection<Arguments> data() throws URISyntaxException, IOException {

driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedRetryableWritesTest.java

@@ -16,6 +16,7 @@
 
 package com.mongodb.client.unified;
 
+import com.mongodb.client.unified.UnifiedTestSkips.TestDef;
 import org.junit.jupiter.params.provider.Arguments;
 
 import java.io.IOException;
@@ -25,24 +26,27 @@
 import static com.mongodb.ClusterFixture.isDiscoverableReplicaSet;
 import static com.mongodb.ClusterFixture.isSharded;
 import static com.mongodb.ClusterFixture.serverVersionLessThan;
-import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static com.mongodb.client.unified.UnifiedTestSkips.testDef;
 
 public final class UnifiedRetryableWritesTest extends UnifiedSyncTest {
     @Override
     protected void skips(final String fileDescription, final String testDescription) {
-        doSkips(testDescription);
+        doSkips(testDef("unified-test-format/retryable-writes", fileDescription, testDescription));
     }
 
-    public static void doSkips(final String description) {
+    public static void doSkips(final TestDef def) {
         if (isSharded() && serverVersionLessThan(5, 0)) {
-            assumeFalse(description.contains("succeeds after WriteConcernError"));
-            assumeFalse(description.contains("succeeds after retryable writeConcernError"));
+            def.skipJira("https://jira.mongodb.org/browse/JAVA-5125")
+                    .testContains("retryable-writes", "succeeds after WriteConcernError")
+                    .testContains("retryable-writes", "succeeds after retryable writeConcernError");
         }
         if (isDiscoverableReplicaSet() && serverVersionLessThan(4, 4)) {
-            assumeFalse(description.equals("RetryableWriteError label is added based on writeConcernError in pre-4.4 mongod response"));
+            def.skipJira("https://jira.mongodb.org/browse/JAVA-5341")
+                    .test("retryable-writes", "retryable-writes insertOne serverErrors", "RetryableWriteError label is added based on writeConcernError in pre-4.4 mongod response");
         }
-        assumeFalse(description.contains("client bulkWrite"), "JAVA-4586");
-        assumeFalse(description.contains("client.clientBulkWrite"), "JAVA-4586");
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-4586")
+                .testContains("retryable-writes", "client bulkWrite")
+                .testContains("retryable-writes", "client.clientBulkWrite");
     }
 
     private static Collection<Arguments> data() throws URISyntaxException, IOException {

driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedServerDiscoveryAndMonitoringTest.java

@@ -22,7 +22,8 @@
 import java.net.URISyntaxException;
 import java.util.Collection;
 
-import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static com.mongodb.client.unified.UnifiedTestSkips.TestDef;
+import static com.mongodb.client.unified.UnifiedTestSkips.testDef;
 
 public final class UnifiedServerDiscoveryAndMonitoringTest extends UnifiedSyncTest {
     private static Collection<Arguments> data() throws URISyntaxException, IOException {
@@ -31,30 +32,26 @@ private static Collection<Arguments> data() throws URISyntaxException, IOExcepti
 
     @Override
     protected void skips(final String fileDescription, final String testDescription) {
-        doSkips(fileDescription, testDescription);
+        doSkips(testDef("unified-test-format/server-discovery-and-monitoring", fileDescription, testDescription));
     }
 
-    public static void doSkips(final String fileDescription, final String testDescription) {
-        assumeFalse(testDescription.equals("connect with serverMonitoringMode=auto >=4.4"),
-                "Skipping because our server monitoring events behave differently for now");
-        assumeFalse(testDescription.equals("connect with serverMonitoringMode=stream >=4.4"),
-                "Skipping because our server monitoring events behave differently for now");
-
-        assumeFalse(fileDescription.equals("standalone-logging"), "Skipping until JAVA-4770 is implemented");
-        assumeFalse(fileDescription.equals("replicaset-logging"), "Skipping until JAVA-4770 is implemented");
-        assumeFalse(fileDescription.equals("sharded-logging"), "Skipping until JAVA-4770 is implemented");
-        assumeFalse(fileDescription.equals("loadbalanced-logging"), "Skipping until JAVA-4770 is implemented");
-
-        assumeFalse(fileDescription.equals("standalone-emit-topology-description-changed-before-close"),
-                "Skipping until JAVA-5229 is implemented");
-        assumeFalse(fileDescription.equals("replicaset-emit-topology-description-changed-before-close"),
-                "Skipping until JAVA-5229 is implemented");
-        assumeFalse(fileDescription.equals("sharded-emit-topology-description-changed-before-close"),
-                "Skipping until JAVA-5229 is implemented");
-        assumeFalse(fileDescription.equals("loadbalanced-emit-topology-description-changed-before-close"),
-                "Skipping until JAVA-5229 is implemented");
-
-        assumeFalse(testDescription.equals("poll waits after successful heartbeat"), "Skipping until JAVA-5564 is implemented");
-        assumeFalse(fileDescription.equals("interruptInUse"), "Skipping until JAVA-4536 is implemented");
+    public static void doSkips(final TestDef def) {
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-5230")
+                .test("server-discovery-and-monitoring", "serverMonitoringMode", "connect with serverMonitoringMode=auto >=4.4")
+                .test("server-discovery-and-monitoring", "serverMonitoringMode", "connect with serverMonitoringMode=stream >=4.4");
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-4770")
+                .file("server-discovery-and-monitoring", "standalone-logging")
+                .file("server-discovery-and-monitoring", "replicaset-logging")
+                .file("server-discovery-and-monitoring", "sharded-logging")
+                .file("server-discovery-and-monitoring", "loadbalanced-logging");
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-5229")
+                .file("server-discovery-and-monitoring", "standalone-emit-topology-description-changed-before-close")
+                .file("server-discovery-and-monitoring", "replicaset-emit-topology-description-changed-before-close")
+                .file("server-discovery-and-monitoring", "sharded-emit-topology-description-changed-before-close")
+                .file("server-discovery-and-monitoring", "loadbalanced-emit-topology-description-changed-before-close");
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-5564")
+                .test("server-discovery-and-monitoring", "serverMonitoringMode", "poll waits after successful heartbeat");
+        def.skipJira("https://jira.mongodb.org/browse/JAVA-4536")
+                .file("server-discovery-and-monitoring", "interruptInUse");
     }
 }

driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestSkips.java

@@ -0,0 +1,124 @@
+/*
+ * Copyright 2008-present MongoDB, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.mongodb.client.unified;
+
+import com.mongodb.assertions.Assertions;
+
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
+
+public final class UnifiedTestSkips {
+
+    private UnifiedTestSkips() {}
+
+    public static TestDef testDef(final String dir, final String file, final String test) {
+        return new TestDef(dir, file, test);
+    }
+
+    public static final class TestDef {
+        private final String dir;
+        private final String file;
+        private final String test;
+
+        private TestDef(final String dir, final String file, final String test) {
+            this.dir = dir;
+            this.file = file;
+            this.test = test;
+        }
+
+        /**
+         * 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
+         */
+        public Skip skipJira(final String skip) {
+            Assertions.assertTrue(skip.startsWith("https://jira.mongodb.org/browse/JAVA-"));
+            return new Skip(this, skip);
+        }
+
+        /**
+         * Test is skipped because the feature under test was deprecated, and
+         * was removed in the Java driver.
+         *
+         * @param skip reason for skipping the test
+         */
+        public Skip skipDeprecated(final String skip) {
+            return new Skip(this, skip);
+        }
+
+        /**
+         * Test is skipped because the Java driver cannot comply with the spec.
+         *
+         * @param skip reason for skipping the test
+         */
+        public Skip skipNoncompliant(final String skip) {
+            return new Skip(this, skip);
+        }
+    }
+
+    public static final class Skip {
+        private final TestDef testDef;
+        private final String reason;
+
+        private Skip(final TestDef testDef, final String reason) {
+            this.testDef = testDef;
+            this.reason = reason;
+        }
+
+        /**
+         * All tests in file under dir skipped.
+         * @param dir the directory name
+         * @param file the test file's "description" field
+         * @return this
+         */
+        public Skip file(final String dir, final String file) {
+            boolean match = ("unified-test-format/" + dir).equals(testDef.dir)
+                    && file.equals(testDef.file);
+            assumeFalse(match, reason);
+            return this;
+        }
+
+        /**
+         * Test skipped if dir, file, and test match.
+         * @param dir the directory name
+         * @param file the test file's "description" field
+         * @param test the individual test's "description" field
+         * @return this
+         */
+        public Skip test(final String dir, final String file, final String test) {
+            boolean match = ("unified-test-format/" + dir).equals(testDef.dir)
+                    && file.equals(testDef.file)
+                    && test.equals(testDef.test);
+            assumeFalse(match, reason);
+            return this;
+        }
+
+        /**
+         * Test skipped if the test description contains the fragment as a substring
+         * Avoid using this, except during development.
+         * @param dir the directory name
+         * @param fragment the substring to check in the test "description" field
+         * @return this
+         */
+        public Skip testContains(final String dir, final String fragment) {
+            boolean match = ("unified-test-format/" + dir).equals(testDef.dir)
+                    && testDef.test.contains(fragment);
+            assumeFalse(match, reason);
+            return this;
+        }
+    }
+}