refactor(router): Split TestContextBuilder namespace parameters

Most of the test cases set up a context with or without namespace
autocreation, while few set a retention period. This removes the
requirement to pass a retention period when enabling autocreation
or vice versa.

router/tests/common/mod.rs

@@ -29,7 +29,7 @@ pub const TEST_TOPIC_ID: i64 = 1;
 pub const TEST_QUERY_POOL_ID: i64 = 1;
 
 /// Common retention period value we'll use in tests
-pub const TEST_RETENTION_PERIOD_NS: Option<i64> = Some(3_600 * 1_000_000_000);
+pub const TEST_RETENTION_PERIOD_NS: i64 = 3_600 * 1_000_000_000;
 
 pub fn test_context() -> TestContextBuilder {
     TestContextBuilder::default()
@@ -42,8 +42,22 @@ pub struct TestContextBuilder {
 }
 
 impl TestContextBuilder {
-    pub fn namespace_autocreate_policy(mut self, policy: NamespaceAutocreatePolicy) -> Self {
-        self.namespace_autocreate_policy = Some(policy);
+    pub fn autocreate_namespace(mut self, enabled: bool) -> Self {
+        self.namespace_autocreate_policy = match self.namespace_autocreate_policy {
+            Some(p) => Some(NamespaceAutocreatePolicy { enabled, ..p }),
+            None => Some(NamespaceAutocreatePolicy::new(true, None)),
+        };
+        self
+    }
+
+    pub fn autocreate_namespace_retention_period_nanos(mut self, period_nanos: i64) -> Self {
+        self.namespace_autocreate_policy = match self.namespace_autocreate_policy {
+            Some(p) => Some(NamespaceAutocreatePolicy {
+                retention_period_nanos: Some(period_nanos),
+                ..p
+            }),
+            None => Some(NamespaceAutocreatePolicy::new(false, Some(period_nanos))),
+        };
         self
     }
 
@@ -67,13 +81,13 @@ impl TestContextBuilder {
 }
 
 #[derive(Debug)]
-pub struct NamespaceAutocreatePolicy {
+struct NamespaceAutocreatePolicy {
     enabled: bool,
     retention_period_nanos: Option<i64>,
 }
 
 impl NamespaceAutocreatePolicy {
-    pub fn new(enabled: bool, retention_period_nanos: Option<i64>) -> Self {
+    fn new(enabled: bool, retention_period_nanos: Option<i64>) -> Self {
         Self {
             enabled,
             retention_period_nanos,
@@ -122,7 +136,7 @@ type HttpDelegateStack = HttpDelegate<
 
 /// A [`router`] stack configured with the various DML handlers using mock catalog backends.
 impl TestContext {
-    pub async fn new(
+    async fn new(
         namespace_autocreate_policy: NamespaceAutocreatePolicy,
         catalog: Arc<dyn Catalog>,
         metrics: Arc<metric::Registry>,

router/tests/grpc.rs

@@ -14,7 +14,7 @@ use router::{
 };
 use tonic::{Code, Request};
 
-use crate::common::{test_context, NamespaceAutocreatePolicy};
+use crate::common::test_context;
 
 pub mod common;
 
@@ -140,10 +140,7 @@ async fn test_namespace_create() {
 #[tokio::test]
 async fn test_namespace_delete() {
     // Initialise a TestContext with implicit namespace creation.
-    let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(true, None))
-        .build()
-        .await;
+    let ctx = test_context().autocreate_namespace(true).build().await;
 
     const RETENTION: i64 = Duration::from_secs(42 * 60 * 60).as_nanos() as _;
 

router/tests/http.rs

@@ -1,7 +1,4 @@
-use crate::common::{
-    test_context, NamespaceAutocreatePolicy, TEST_QUERY_POOL_ID, TEST_RETENTION_PERIOD_NS,
-    TEST_TOPIC_ID,
-};
+use crate::common::{test_context, TEST_QUERY_POOL_ID, TEST_RETENTION_PERIOD_NS, TEST_TOPIC_ID};
 use assert_matches::assert_matches;
 use data_types::{ColumnType, QueryPoolId, TopicId};
 use futures::{stream::FuturesUnordered, StreamExt};
@@ -19,10 +16,7 @@ pub mod common;
 #[tokio::test]
 async fn test_write_ok() {
     // Create a test context with implicit namespace creation.
-    let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(true, None))
-        .build()
-        .await;
+    let ctx = test_context().autocreate_namespace(true).build().await;
 
     // Write data inside retention period
     let now = SystemProvider::default()
@@ -97,10 +91,8 @@ async fn test_write_ok() {
 #[tokio::test]
 async fn test_write_outside_retention_period() {
     let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(
-            true,
-            TEST_RETENTION_PERIOD_NS,
-        ))
+        .autocreate_namespace(true)
+        .autocreate_namespace_retention_period_nanos(TEST_RETENTION_PERIOD_NS)
         .build()
         .await;
 
@@ -128,10 +120,7 @@ async fn test_write_outside_retention_period() {
 
 #[tokio::test]
 async fn test_schema_conflict() {
-    let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(true, None))
-        .build()
-        .await;
+    let ctx = test_context().autocreate_namespace(true).build().await;
 
     // data inside the retention period
     let now = SystemProvider::default()
@@ -210,10 +199,7 @@ async fn test_rejected_ns() {
 
 #[tokio::test]
 async fn test_schema_limit() {
-    let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(true, None))
-        .build()
-        .await;
+    let ctx = test_context().autocreate_namespace(true).build().await;
 
     let now = SystemProvider::default()
         .now()
@@ -267,10 +253,7 @@ async fn test_schema_limit() {
 
 #[tokio::test]
 async fn test_write_propagate_ids() {
-    let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(true, None))
-        .build()
-        .await;
+    let ctx = test_context().autocreate_namespace(true).build().await;
 
     // Create the namespace and a set of tables.
     let ns = ctx
@@ -355,10 +338,7 @@ async fn test_write_propagate_ids() {
 
 #[tokio::test]
 async fn test_delete_unsupported() {
-    let ctx = test_context()
-        .namespace_autocreate_policy(NamespaceAutocreatePolicy::new(true, None))
-        .build()
-        .await;
+    let ctx = test_context().autocreate_namespace(true).build().await;
 
     // Create the namespace and a set of tables.
     ctx.catalog()