refactor: Extract a macro for matching errors in tests

Cargo.lock

@@ -1783,6 +1783,7 @@ dependencies = [
  "object_store",
  "observability_deps",
  "snafu",
+ "test_helpers",
  "tokio",
  "tokio-stream",
  "uuid",

iox_object_store/Cargo.toml

@@ -15,3 +15,6 @@ snafu = "0.6"
 tokio = { version = "1.13", features = ["macros", "time"] }
 tokio-stream = "0.1"
 uuid = { version = "0.8", features = ["serde", "v4"] }
+
+[dev-dependencies] # In alphabetical order
+test_helpers = { path = "../test_helpers" }

iox_object_store/src/lib.rs

@@ -478,6 +478,7 @@ mod tests {
     use crate::paths::ALL_DATABASES_DIRECTORY;
     use data_types::chunk_metadata::{ChunkAddr, ChunkId};
     use object_store::{parsed_path, path::ObjectStorePath, ObjectStore, ObjectStoreApi};
+    use test_helpers::assert_error;
     use uuid::Uuid;
 
     /// Creates a new in-memory object store
@@ -817,15 +818,9 @@ mod tests {
             .await
             .unwrap();
 
-        let err = IoxObjectStore::create(Arc::clone(&object_store), uuid)
+        assert_error!(
-            .await
+            IoxObjectStore::create(Arc::clone(&object_store), uuid).await,
-            .unwrap_err();
+            IoxObjectStoreError::DatabaseAlreadyExists { uuid: err_uuid } if err_uuid == uuid,
-        assert!(
-            matches!(
-                err,
-                IoxObjectStoreError::DatabaseAlreadyExists { uuid: err_uuid } if err_uuid == uuid),
-            "got: {:?}",
-            err
         );
     }
 
@@ -842,15 +837,9 @@ mod tests {
             .await
             .unwrap();
 
-        let err = IoxObjectStore::create(Arc::clone(&object_store), uuid)
+        assert_error!(
-            .await
+            IoxObjectStore::create(Arc::clone(&object_store), uuid).await,
-            .unwrap_err();
+            IoxObjectStoreError::DatabaseAlreadyExists { uuid: err_uuid } if err_uuid == uuid,
-        assert!(
-            matches!(
-                err,
-                IoxObjectStoreError::DatabaseAlreadyExists { uuid: err_uuid } if err_uuid == uuid),
-            "got: {:?}",
-            err
         );
     }
 
@@ -869,15 +858,9 @@ mod tests {
             .unwrap();
         iox_object_store.write_tombstone().await.unwrap();
 
-        let err = IoxObjectStore::create(Arc::clone(&object_store), uuid)
+        assert_error!(
-            .await
+            IoxObjectStore::create(Arc::clone(&object_store), uuid).await,
-            .unwrap_err();
+            IoxObjectStoreError::DatabaseAlreadyExists { uuid: err_uuid } if err_uuid == uuid,
-        assert!(
-            matches!(
-                err,
-                IoxObjectStoreError::DatabaseAlreadyExists { uuid: err_uuid } if err_uuid == uuid),
-            "got: {:?}",
-            err
         );
     }
 
@@ -970,14 +953,9 @@ mod tests {
         let db = Uuid::new_v4();
 
         // This fails, there are no rules to read
-        let err = IoxObjectStore::load_database_rules(object_store, db)
+        assert_error!(
-            .await
+            IoxObjectStore::load_database_rules(object_store, db).await,
-            .unwrap_err();
+            object_store::Error::NotFound { .. },
-
-        assert!(
-            matches!(err, object_store::Error::NotFound { .. }),
-            "got: {:?}",
-            err
         );
     }
 
@@ -987,13 +965,9 @@ mod tests {
 
         // Load can't find nonexistent database
         let nonexistent = Uuid::new_v4();
-        let returned = IoxObjectStore::load(Arc::clone(&object_store), nonexistent)
+        assert_error!(
-            .await
+            IoxObjectStore::load(Arc::clone(&object_store), nonexistent).await,
-            .unwrap_err();
+            IoxObjectStoreError::NoRulesFound { .. },
-        assert!(
-            matches!(returned, IoxObjectStoreError::NoRulesFound { .. }),
-            "got: {:?}",
-            returned
         );
 
         // Create a database
@@ -1013,13 +987,9 @@ mod tests {
         delete_database(&db_iox_store).await;
 
         // Load can't find deleted database
-        let returned = IoxObjectStore::load(Arc::clone(&object_store), db)
+        assert_error!(
-            .await
+            IoxObjectStore::load(Arc::clone(&object_store), db).await,
-            .unwrap_err();
+            IoxObjectStoreError::DatabaseDeleted { .. },
-        assert!(
-            matches!(returned, IoxObjectStoreError::DatabaseDeleted { .. }),
-            "got: {:?}",
-            returned
         );
     }
 

iox_object_store/src/paths/parquet_file.rs

@@ -139,6 +139,7 @@ mod tests {
     use super::*;
     use crate::{paths::ALL_DATABASES_DIRECTORY, IoxObjectStore, RootPath};
     use object_store::{ObjectStore, ObjectStoreApi};
+    use test_helpers::assert_error;
 
     /// Creates a new in-memory object store. These tests rely on the `Path`s being of type
     /// `DirsAndFileName` and thus using object_store::path::DELIMITER as the separator
@@ -250,18 +251,12 @@ mod tests {
         path.push_all_dirs(&["server", "uuid", "data", "}*", "aoeu"]);
         // but this file name doesn't contain a chunk id
         path.set_file_name("rules.pb");
-        let result = ParquetFilePath::from_absolute(path);
+        assert_error!(ParquetFilePath::from_absolute(path), InvalidChunkId { .. });
-        assert!(
-            matches!(result, Err(InvalidChunkId { .. })),
-            "got: {:?}",
-            result
-        );
 
         let mut path = object_store.new_path();
         path.push_all_dirs(&["server", "uuid", "data", "}*", "aoeu"]);
         // missing file name
-        let result = ParquetFilePath::from_absolute(path);
+        assert_error!(ParquetFilePath::from_absolute(path), MissingChunkId);
-        assert!(matches!(result, Err(MissingChunkId)), "got: {:?}", result);
     }
 
     #[test]

iox_object_store/src/paths/transaction_file.rs

@@ -198,6 +198,7 @@ mod tests {
     use crate::{paths::ALL_DATABASES_DIRECTORY, IoxObjectStore, RootPath};
     use object_store::{ObjectStore, ObjectStoreApi};
     use std::sync::Arc;
+    use test_helpers::assert_error;
 
     /// Creates a new in-memory object store. These tests rely on the `Path`s being of type
     /// `DirsAndFileName` and thus using object_store::path::DELIMITER as the separator
@@ -324,18 +325,15 @@ mod tests {
         // right directories so we don't check again on the way out
         path.push_all_dirs(&["foo", "bar", "baz", "}*", "aoeu", "blah"]);
         path.set_file_name("rules.pb");
-        let result = TransactionFilePath::from_absolute(path);
+        assert_error!(
-        assert!(
+            TransactionFilePath::from_absolute(path),
-            matches!(result, Err(InvalidRevisionCounter { .. })),
+            InvalidRevisionCounter { .. },
-            "got: {:?}",
-            result
         );
 
         let mut path = object_store.new_path();
         path.push_all_dirs(&["dbs", "uuid", "data", "00000000000000000123"]);
         // missing file name
-        let result = TransactionFilePath::from_absolute(path);
+        assert_error!(TransactionFilePath::from_absolute(path), MissingFileName,);
-        assert!(matches!(result, Err(MissingFileName)), "got: {:?}", result);
     }
 
     #[test]

server/src/db.rs

@@ -1341,7 +1341,7 @@ mod tests {
         },
         utils::{make_db, make_db_time, TestDb},
     };
-    use ::test_helpers::assert_contains;
+    use ::test_helpers::{assert_contains, assert_error};
     use arrow::record_batch::RecordBatch;
     use arrow_util::{assert_batches_eq, assert_batches_sorted_eq};
     use bytes::Bytes;
@@ -1481,11 +1481,7 @@ mod tests {
         let write = DbWrite::new(tables, Default::default());
         let res = db.route_write(&write).await;
 
-        assert!(
+        assert_error!(res, Error::WriteBufferWritingError { .. });
-            matches!(res, Err(Error::WriteBufferWritingError { .. })),
-            "Expected Err(Error::WriteBufferWritingError {{ .. }}), got: {:?}",
-            res
-        );
     }
 
     #[tokio::test]

server/src/lib.rs

@@ -1438,7 +1438,7 @@ mod tests {
         },
         time::{Duration, Instant},
     };
-    use test_helpers::assert_contains;
+    use test_helpers::{assert_contains, assert_error};
 
     #[tokio::test]
     async fn server_api_calls_return_error_with_no_id_set() {
@@ -2377,16 +2377,11 @@ mod tests {
 
         let nonexistent_uuid = Uuid::new_v4();
 
-        let err = server.restore_database(nonexistent_uuid).await.unwrap_err();
+        assert_error!(
-        assert!(
+            server.restore_database(nonexistent_uuid).await,
-            matches!(
+            Error::CouldNotGetDatabaseNameFromRules {
-                err,
+                source: DatabaseNameFromRulesError::DatabaseRulesNotFound { .. },
-                Error::CouldNotGetDatabaseNameFromRules {
+            }
-                    source: DatabaseNameFromRulesError::DatabaseRulesNotFound { .. },
-                }
-            ),
-            "got: {:?}",
-            err
         );
     }
 
@@ -2421,11 +2416,9 @@ mod tests {
             .expect("failed to create database");
 
         // trying to restore the first foo fails
-        let err = server.restore_database(first_foo_uuid).await.unwrap_err();
+        assert_error!(
-        assert!(
+            server.restore_database(first_foo_uuid).await,
-            matches!(err, Error::DatabaseAlreadyExists { .. }),
+            Error::DatabaseAlreadyExists { .. },
-            "got: {:?}",
-            err
         );
     }
 
@@ -2458,11 +2451,9 @@ mod tests {
         server.restore_database(foo_uuid).await.unwrap();
 
         // restoring again fails
-        let err = server.restore_database(foo_uuid).await.unwrap_err();
+        assert_error!(
-        assert!(
+            server.restore_database(foo_uuid).await,
-            matches!(err, Error::DatabaseAlreadyActive { .. }),
+            Error::DatabaseAlreadyActive { .. },
-            "got: {:?}",
-            err
         );
     }
 
@@ -2709,12 +2700,9 @@ mod tests {
 
         let tables = lines_to_batches("cpu bar=1 10", 0).unwrap();
         let write = DbWrite::new(tables, Default::default());
-        let err = server.write(&db_name, write).await.unwrap_err();
+        assert_error!(
-
+            server.write(&db_name, write).await,
-        assert!(
+            Error::WriteBuffer { .. },
-            matches!(err, Error::WriteBuffer { .. }),
-            "Expected Err(Error::WriteBuffer {{ .. }}), got: {:?}",
-            err
         );
     }
 

test_helpers/src/lib.rs

@@ -146,3 +146,13 @@ macro_rules! assert_not_contains {
         );
     };
 }
+
+#[macro_export]
+/// Assert that an operation fails with one particular error. Panics if the operation succeeds.
+/// Prints debug format of the error value if it doesn't match the specified pattern.
+macro_rules! assert_error {
+    ($OPERATION: expr, $(|)? $( $ERROR_PATTERN:pat_param )|+ $( if $GUARD: expr )? $(,)?) => {
+        let err = $OPERATION.unwrap_err();
+        assert!(matches!(err, $( $ERROR_PATTERN )|+ $( if $GUARD )?), "got: {:?}", err);
+    };
+}