Refactor spec test parser and runners (#841)

Author: isabelatkinson

src/test/atlas_planned_maintenance_testing/mod.rs

@@ -80,7 +80,7 @@ async fn execute_workload(test_runner: &mut TestRunner, workload: Value) -> Vec<
 
     log_uncaptured("Running planned maintenance tests");
 
-    if AssertUnwindSafe(test_runner.run_test(None, test_file, |_| true))
+    if AssertUnwindSafe(test_runner.run_test(test_file, None, None))
         .catch_unwind()
         .await
         .is_err()

src/test/mod.rs

@@ -21,7 +21,7 @@ pub(crate) mod util;
 #[cfg(feature = "in-use-encryption-unstable")]
 pub(crate) use self::csfle::{KmsProviderList, KMS_PROVIDERS_MAP};
 pub(crate) use self::{
-    spec::{run_single_test, run_spec_test, run_spec_test_with_path, RunOn, Serverless, Topology},
+    spec::{run_spec_test, RunOn, Serverless, Topology},
     util::{
         assert_matches,
         eq_matches,

src/test/spec/change_streams.rs

@@ -1,10 +1,8 @@
-use crate::test::LOCK;
-
-use super::{run_spec_test_with_path, run_unified_format_test};
+use crate::test::{spec::unified_runner::run_unified_tests, LOCK};
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test(flavor = "multi_thread"))] // multi_thread required for FailPoint
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
-async fn run() {
+async fn run_unified() {
     let _guard = LOCK.run_exclusively().await;
-    run_spec_test_with_path(&["change-streams", "unified"], run_unified_format_test).await;
+    run_unified_tests(&["change-streams", "unified"]).await;
 }

src/test/spec/client_side_encryption.rs

@@ -1,62 +1,37 @@
 use tokio::sync::RwLockWriteGuard;
 
-use crate::test::{log_uncaptured, LOCK};
-
-use super::{
-    run_spec_test_with_path,
-    run_unified_format_test_filtered,
-    run_v2_test,
-    unified_runner::TestCase,
+use crate::test::{
+    spec::{unified_runner::run_unified_tests, v2_runner::run_v2_tests},
+    LOCK,
 };
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test(flavor = "multi_thread"))]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
 async fn run_unified() {
     let _guard: RwLockWriteGuard<()> = LOCK.run_exclusively().await;
-    run_spec_test_with_path(&["client-side-encryption", "unified"], |path, test| {
-        run_unified_format_test_filtered(path, test, spec_predicate)
-    })
-    .await;
-}
 
-#[allow(unused_variables)]
-fn spec_predicate(test: &TestCase) -> bool {
     #[cfg(not(feature = "openssl-tls"))]
-    {
-        if test.description == "create datakey with KMIP KMS provider" {
-            crate::test::log_uncaptured(format!(
-                "Skipping {:?}: KMIP test requires openssl-tls",
-                test.description
-            ));
-            return false;
-        }
-    }
-    true
+    let skipped_tests = &["create datakey with KMIP KMS provider"];
+    #[cfg(feature = "openssl-tls")]
+    let skipped_tests = &[];
+
+    run_unified_tests(&["client-side-encryption", "unified"])
+        .skip_tests(skipped_tests)
+        .await;
 }
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test(flavor = "multi_thread"))]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
 async fn run_legacy() {
     let _guard: RwLockWriteGuard<()> = LOCK.run_exclusively().await;
 
-    run_spec_test_with_path(&["client-side-encryption", "legacy"], |path, test| async {
-        // TODO RUST-582 Unskip this when we have csot implemented.
-        if path.ends_with("timeoutMS.json") {
-            log_uncaptured(format!(
-                "Skipping {}: requires client side operations timeout",
-                path.display()
-            ));
-            return;
-        }
-        #[cfg(not(feature = "openssl-tls"))]
-        if path.ends_with("kmipKMS.json") {
-            log_uncaptured(format!(
-                "Skipping {}: KMIP requires openssl",
-                path.display()
-            ));
-            return;
-        }
-        run_v2_test(path, test).await;
-    })
-    .await;
+    // TODO RUST-528: Unskip timeoutMS.json when CSOT is implemented.
+    #[cfg(not(feature = "openssl-tls"))]
+    let skipped_files = &["timeoutMS.json", "kmipKMS.json"];
+    #[cfg(feature = "openssl-tls")]
+    let skipped_files = &["timeoutMS.json"];
+
+    run_v2_tests(&["client-side-encryption", "legacy"])
+        .skip_files(skipped_files)
+        .await;
 }

src/test/spec/collection_management.rs

@@ -1,18 +1,11 @@
-use crate::test::{log_uncaptured, LOCK};
-
-use super::{run_spec_test_with_path, run_unified_format_test};
+use crate::test::{spec::unified_runner::run_unified_tests, LOCK};
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test)]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
-async fn run() {
+async fn run_unified() {
     let _guard = LOCK.run_exclusively().await;
-    run_spec_test_with_path(&["collection-management"], |path, test| async {
-        if path.ends_with("modifyCollection-pre_and_post_images.json") {
-            // modifyCollection is unsupported.
-            log_uncaptured("skipping modifyCollection-pre_and_post_images");
-            return;
-        }
-        run_unified_format_test(path, test).await;
-    })
-    .await;
+    run_unified_tests(&["collection-management"])
+        // The driver does not support modifyCollection.
+        .skip_files(&["modifyCollection-pre_and_post_images.json"])
+        .await;
 }

src/test/spec/command_monitoring/mod.rs

@@ -1,19 +1,14 @@
-use crate::test::LOCK;
-
-use super::{run_spec_test_with_path, run_unified_format_test_filtered, unified_runner::TestCase};
+use crate::test::{spec::unified_runner::run_unified_tests, LOCK};
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test(flavor = "multi_thread"))]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
 async fn command_monitoring_unified() {
     let _guard = LOCK.run_exclusively().await;
-
-    let pred = |tc: &TestCase|
-        // This test relies on old OP_QUERY behavior that many drivers still use for < 4.4, but we do not use, due to never implementing OP_QUERY.
-        tc.description != "A successful find event with a getmore and the server kills the cursor (<= 4.4)";
-
-    run_spec_test_with_path(
-        &["command-logging-and-monitoring", "monitoring"],
-        |path, f| run_unified_format_test_filtered(path, f, pred),
-    )
-    .await;
+    run_unified_tests(&["command-logging-and-monitoring", "monitoring"])
+        .skip_tests(&[
+            // This test relies on old OP_QUERY behavior that many drivers still use for < 4.4, but
+            // we do not use, due to never implementing OP_QUERY.
+            "A successful find event with a getmore and the server kills the cursor (<= 4.4)",
+        ])
+        .await;
 }

src/test/spec/crud.rs

@@ -1,27 +1,40 @@
 use tokio::sync::RwLockWriteGuard;
 
-use crate::test::LOCK;
-
-use super::{run_spec_test_with_path, run_unified_format_test_filtered, unified_runner::TestCase};
+use crate::test::{spec::unified_runner::run_unified_tests, LOCK};
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test(flavor = "multi_thread"))]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
-async fn run() {
+async fn run_unified() {
     let _guard: RwLockWriteGuard<()> = LOCK.run_exclusively().await;
-    run_spec_test_with_path(&["crud", "unified"], |path, file| {
-        run_unified_format_test_filtered(path, file, test_predicate)
-    })
-    .await;
-}
-
-fn test_predicate(test: &TestCase) -> bool {
-    // The Rust driver doesn't support unacknowledged writes.
-    let lower = test.description.to_lowercase();
-
-    !lower.contains("unacknowledged")
-        // TODO: RUST-663: unskip aggregate $out and $merge tests
-        && !(lower.contains("aggregate with $out includes read preference for 5.0+ server"))
-        && !(lower.contains("aggregate with $out omits read preference for pre-5.0 server"))
-        && !(lower.contains("aggregate with $merge includes read preference for 5.0+ server"))
-        && !(lower.contains("aggregate with $merge omits read preference for pre-5.0 server"))
+    run_unified_tests(&["crud", "unified"])
+        .skip_files(&[
+            // The Rust driver does not support unacknowledged writes (and does not intend to in
+            // the future).
+            "bulkWrite-deleteMany-hint-unacknowledged.json",
+            "bulkWrite-deleteOne-hint-unacknowledged.json",
+            "bulkWrite-replaceOne-hint-unacknowledged.json",
+            "bulkWrite-updateMany-hint-unacknowledged.json",
+            "bulkWrite-updateOne-hint-unacknowledged.json",
+            "deleteMany-hint-unacknowledged.json",
+            "deleteOne-hint-unacknowledged.json",
+            "findOneAndDelete-hint-unacknowledged.json",
+            "findOneAndReplace-hint-unacknowledged.json",
+            "findOneAndUpdate-hint-unacknowledged.json",
+            "replaceOne-hint-unacknowledged.json",
+            "updateMany-hint-unacknowledged.json",
+            "updateOne-hint-unacknowledged.json",
+        ])
+        .skip_tests(&[
+            // Unacknowledged write; see above.
+            "Unacknowledged write using dollar-prefixed or dotted keys may be silently rejected \
+             on pre-5.0 server",
+            // TODO RUST-663: Unskip these tests.
+            "Aggregate with $out includes read preference for 5.0+ server",
+            "Aggregate with $out omits read preference for pre-5.0 server",
+            "Aggregate with $merge includes read preference for 5.0+ server",
+            "Aggregate with $merge omits read preference for pre-5.0 server",
+            "Database-level aggregate with $out omits read preference for pre-5.0 server",
+            "Database-level aggregate with $merge omits read preference for pre-5.0 server",
+        ])
+        .await;
 }

src/test/spec/gridfs.rs

@@ -9,8 +9,7 @@ use crate::{
     options::{GridFsBucketOptions, GridFsUploadOptions},
     runtime,
     test::{
-        run_spec_test_with_path,
-        spec::unified_runner::{run_unified_format_test_filtered, TestCase},
+        spec::unified_runner::run_unified_tests,
         FailCommandOptions,
         FailPoint,
         FailPointMode,
@@ -22,19 +21,14 @@ use crate::{
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test)]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
-async fn run() {
+async fn run_unified() {
     let _guard = LOCK.run_concurrently().await;
-    run_spec_test_with_path(&["gridfs"], |path, f| {
-        run_unified_format_test_filtered(path, f, test_predicate)
-    })
-    .await;
-}
-
-fn test_predicate(test: &TestCase) -> bool {
-    let lower = test.description.to_lowercase();
-
-    // The Rust driver doesn't support the disableMD5 and contentType options for upload.
-    !lower.contains("sans md5") && !lower.contains("contenttype")
+    run_unified_tests(&["gridfs"])
+        // The Rust driver doesn't support the disableMD5 option.
+        .skip_files(&["upload-disableMD5.json"])
+        // The Rust driver doesn't support the contentType option.
+        .skip_tests(&["upload when contentType is provided"])
+        .await;
 }
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test)]

src/test/spec/load_balancers.rs

@@ -1,29 +1,23 @@
-use std::path::PathBuf;
-
 use tokio::sync::RwLockWriteGuard;
 
 use crate::test::{
-    run_spec_test_with_path,
     spec::{
-        run_unified_format_test,
-        unified_runner::{ExpectedCmapEvent, ExpectedEvent, TestFile},
+        unified_runner::{run_unified_tests, ExpectedCmapEvent, ExpectedEvent, TestFile},
         ExpectedEventType,
     },
     LOCK,
 };
 
 #[cfg_attr(feature = "tokio-runtime", tokio::test(flavor = "multi_thread"))]
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
-async fn run() {
-    async fn run_test(path: PathBuf, mut test_file: TestFile) {
-        // The Rust driver doesn't support wait queue timeouts.
-        if path.ends_with("wait-queue-timeouts.json") {
-            return;
-        }
-        // The Rust driver's asynchronous check-in of connections means that sometimes a new
-        // connection will be created, rather than re-using an existing one; the connectionReady
-        // events generated by this cause tests to fail, but does not break user-visible behavior.
-        // TODO RUST-1055 Remove this workaround.
+async fn run_unified() {
+    let _guard: RwLockWriteGuard<()> = LOCK.run_exclusively().await;
+
+    // The Rust driver's asynchronous check-in of connections means that sometimes a new
+    // connection will be created, rather than re-using an existing one; the connectionReady
+    // events generated by this cause tests to fail, but does not break user-visible behavior.
+    // TODO RUST-1055 Remove this workaround.
+    fn file_transformation(test_file: &mut TestFile) {
         for test in &mut test_file.tests {
             if let Some(e) = test.expect_events.as_mut() {
                 for expect_events in e {
@@ -40,8 +34,11 @@ async fn run() {
                 }
             }
         }
-        run_unified_format_test(path, test_file).await;
     }
-    let _guard: RwLockWriteGuard<()> = LOCK.run_exclusively().await;
-    run_spec_test_with_path(&["load-balancers"], run_test).await;
+
+    run_unified_tests(&["load-balancers"])
+        // The Rust driver doesn't support wait queue timeouts.
+        .skip_files(&["wait-queue-timeouts.json"])
+        .transform_files(file_transformation)
+        .await;
 }