From 6cf180738be8c8c297e4429e7d9af45de24a8522 Mon Sep 17 00:00:00 2001
From: Dom Dwyer <dom@itsallbroken.com>
Date: Tue, 23 May 2023 12:04:52 +0200
Subject: [PATCH] test: more exacting retention validation tests

The old tests used partial error string matching, with the whole error
message! So when I added more to the error message, the fixture tests
didn't fail.

This changes the tests to match the full string, and validate the
timestamps are included.
---
 .../src/dml_handlers/retention_validation.rs  | 94 +++++++++++--------
 1 file changed, 57 insertions(+), 37 deletions(-)

diff --git a/router/src/dml_handlers/retention_validation.rs b/router/src/dml_handlers/retention_validation.rs
index eb612e2ae5..a4607a6eff 100644
--- a/router/src/dml_handlers/retention_validation.rs
+++ b/router/src/dml_handlers/retention_validation.rs
@@ -49,7 +49,10 @@ impl RetentionValidator {
 }
 
 #[async_trait]
-impl DmlHandler for RetentionValidator {
+impl<P> DmlHandler for RetentionValidator<P>
+where
+    P: TimeProvider,
+{
     type WriteError = RetentionError;
 
     type WriteInput = HashMap<String, MutableBatch>;
@@ -88,7 +91,9 @@ impl DmlHandler for RetentionValidator {
 mod tests {
     use std::sync::Arc;
 
+    use assert_matches::assert_matches;
     use iox_tests::{TestCatalog, TestNamespace};
+    use iox_time::MockProvider;
     use once_cell::sync::Lazy;
 
     use super::*;
@@ -113,12 +118,10 @@ mod tests {
         let line = "bananas,tag1=A,tag2=B val=42i ".to_string() + &now;
         let writes = lp_to_writes(&line);
 
-        let result = handler
+        let _result = handler
             .write(&NAMESPACE, namespace.schema().await.into(), writes, None)
-            .await;
-
-        // no error means the time is inside the retention period
-        assert!(result.is_ok());
+            .await
+            .unwrap();
     }
 
     #[tokio::test]
@@ -128,13 +131,16 @@ mod tests {
         // Create the table so that there is a known ID that must be returned.
         let _want_id = namespace.create_table("bananas").await.table.id;
 
-        // Create the validator whose retention period is 1 hour
-        let handler = RetentionValidator::new();
+        let mock_now = iox_time::Time::from_rfc3339("2023-05-23T09:59:06+00:00").unwrap();
+        let mock_time = MockProvider::new(mock_now);
+
+        // Create the validator whse retention period is 1 hour
+        let handler = RetentionValidator {
+            time_provider: mock_time.clone(),
+        };
 
         // Make time outside the retention period
-        let two_hours_ago = (SystemProvider::default().now().timestamp_nanos()
-            - 2 * 3_600 * 1_000_000_000)
-            .to_string();
+        let two_hours_ago = (mock_now.timestamp_nanos() - 2 * 3_600 * 1_000_000_000).to_string();
         let line = "bananas,tag1=A,tag2=B val=42i ".to_string() + &two_hours_ago;
         let writes = lp_to_writes(&line);
 
@@ -143,9 +149,14 @@ mod tests {
             .await;
 
         // error means the time is outside the retention period
-        assert!(result.is_err());
-        let message = result.unwrap_err().to_string();
-        assert!(message.contains("data in table bananas is outside of the retention period"));
+        assert_matches!(result, Err(e) => {
+            assert_eq!(
+                e.to_string(),
+                "data in table bananas is outside of the retention period: \
+                minimum acceptable timestamp is 2023-05-23T08:59:06+00:00, but \
+                observed timestamp 2023-05-23T07:59:06+00:00 is older."
+            )
+        });
     }
 
     #[tokio::test]
@@ -155,19 +166,19 @@ mod tests {
         // Create the table so that there is a known ID that must be returned.
         let _want_id = namespace.create_table("bananas").await.table.id;
 
-        // Create the validator whose retention period is 1 hour
-        let handler = RetentionValidator::new();
+        let mock_now = iox_time::Time::from_rfc3339("2023-05-23T09:59:06+00:00").unwrap();
+        let mock_time = MockProvider::new(mock_now);
+
+        // Create the validator whse retention period is 1 hour
+        let handler = RetentionValidator {
+            time_provider: mock_time.clone(),
+        };
 
         // Make time now to be inside the retention period
-        let now = SystemProvider::default()
-            .now()
-            .timestamp_nanos()
-            .to_string();
+        let now = mock_now.timestamp_nanos().to_string();
         let line1 = "bananas,tag1=A,tag2=B val=42i ".to_string() + &now;
         // Make time outside the retention period
-        let two_hours_ago = (SystemProvider::default().now().timestamp_nanos()
-            - 2 * 3_600 * 1_000_000_000)
-            .to_string();
+        let two_hours_ago = (mock_now.timestamp_nanos() - 2 * 3_600 * 1_000_000_000).to_string();
         let line2 = "bananas,tag1=AA,tag2=BB val=422i ".to_string() + &two_hours_ago;
         // a lp with 2 lines, one inside and one outside retention period
         let lp = format!("{line1}\n{line2}");
@@ -178,9 +189,14 @@ mod tests {
             .await;
 
         // error means the time is outside the retention period
-        assert!(result.is_err());
-        let message = result.unwrap_err().to_string();
-        assert!(message.contains("data in table bananas is outside of the retention period"));
+        assert_matches!(result, Err(e) => {
+            assert_eq!(
+                e.to_string(),
+                "data in table bananas is outside of the retention period: \
+                minimum acceptable timestamp is 2023-05-23T08:59:06+00:00, but \
+                observed timestamp 2023-05-23T07:59:06+00:00 is older."
+            )
+        });
     }
 
     #[tokio::test]
@@ -190,19 +206,19 @@ mod tests {
         // Create the table so that there is a known ID that must be returned.
         let _want_id = namespace.create_table("bananas").await.table.id;
 
+        let mock_now = iox_time::Time::from_rfc3339("2023-05-23T09:59:06+00:00").unwrap();
+        let mock_time = MockProvider::new(mock_now);
+
         // Create the validator whse retention period is 1 hour
-        let handler = RetentionValidator::new();
+        let handler = RetentionValidator {
+            time_provider: mock_time.clone(),
+        };
 
         // Make time now to be inside the retention period
-        let now = SystemProvider::default()
-            .now()
-            .timestamp_nanos()
-            .to_string();
+        let now = mock_now.timestamp_nanos().to_string();
         let line1 = "bananas,tag1=A,tag2=B val=42i ".to_string() + &now;
         // Make time outside the retention period
-        let two_hours_ago = (SystemProvider::default().now().timestamp_nanos()
-            - 2 * 3_600 * 1_000_000_000)
-            .to_string();
+        let two_hours_ago = (mock_now.timestamp_nanos() - 2 * 3_600 * 1_000_000_000).to_string();
         let line2 = "apple,tag1=AA,tag2=BB val=422i ".to_string() + &two_hours_ago;
         // a lp with 2 lines, one inside and one outside retention period
         let lp = format!("{line1}\n{line2}");
@@ -213,9 +229,13 @@ mod tests {
             .await;
 
         // error means the time is outside the retention period
-        assert!(result.is_err());
-        let message = result.unwrap_err().to_string();
-        assert!(message.contains("data in table apple is outside of the retention period"));
+        assert_matches!(result, Err(e) => {
+            assert_eq!(
+                e.to_string(),
+                "data in table apple is outside of the retention period: minimum \
+                 acceptable timestamp is 2023-05-23T08:59:06+00:00, but observed \
+                 timestamp 2023-05-23T07:59:06+00:00 is older.")
+        });
     }
 
     // Parse `lp` into a table-keyed MutableBatch map.