Rework tests

Signed-off-by: Ryan Levick <[email protected]>

Author: rylev

crates/factor-key-value/Cargo.toml

@@ -19,8 +19,8 @@ tracing = { workspace = true }
 
 [dev-dependencies]
 spin-factors-test = { path = "../factors-test" }
-spin-key-value-redis = { path = "../key-value-redis" }
 spin-key-value-spin = { path = "../key-value-spin" }
+spin-key-value-redis = { path = "../key-value-redis" }
 tempfile = "3.12.0"
 tokio = { version = "1", features = ["macros", "rt"] }
 

crates/factor-key-value/src/runtime_config.rs

@@ -18,6 +18,11 @@ impl RuntimeConfig {
     pub fn add_store_manager(&mut self, label: String, store_manager: Arc<dyn StoreManager>) {
         self.store_managers.insert(label, store_manager);
     }
+
+    /// Returns whether a store manager exists for the store with the given label.
+    pub fn has_store_manager(&self, label: &str) -> bool {
+        self.store_managers.contains_key(label)
+    }
 }
 
 impl IntoIterator for RuntimeConfig {

crates/factor-key-value/src/runtime_config/spin.rs

@@ -100,7 +100,23 @@ impl RuntimeConfigResolver {
     /// Resolves a toml table into a runtime config.
     ///
     /// The default stores are also added to the runtime config.
-    pub fn resolve_from_toml(
+    pub fn resolve(&self, table: Option<&impl GetTomlValue>) -> anyhow::Result<RuntimeConfig> {
+        let mut runtime_config = self.resolve_from_toml(table)?.unwrap_or_default();
+
+        for (&label, config) in &self.defaults {
+            if !runtime_config.store_managers.contains_key(label) {
+                let store_manager = self
+                    .store_manager_from_config(config.clone())
+                    .with_context(|| {
+                        format!("could not configure key-value store with label '{label}'")
+                    })?;
+                runtime_config.add_store_manager(label.to_owned(), store_manager);
+            }
+        }
+        Ok(runtime_config)
+    }
+
+    fn resolve_from_toml(
         &self,
         table: Option<&impl GetTomlValue>,
     ) -> anyhow::Result<Option<RuntimeConfig>> {
@@ -117,16 +133,6 @@ impl RuntimeConfigResolver {
             runtime_config.add_store_manager(label.clone(), store_manager);
         }
 
-        for (&label, config) in &self.defaults {
-            if !runtime_config.store_managers.contains_key(label) {
-                let store_manager = self
-                    .store_manager_from_config(config.clone())
-                    .with_context(|| {
-                        format!("could not configure key-value store with label '{label}'")
-                    })?;
-                runtime_config.add_store_manager(label.to_owned(), store_manager);
-            }
-        }
         Ok(Some(runtime_config))
     }
 

crates/factor-key-value/tests/factor_test.rs

@@ -141,159 +141,3 @@ impl Store for MockStore {
         todo!()
     }
 }
-
-// async fn run_test_with_config_and_stores_for_label(
-//     runtime_config: Option<toml::Table>,
-//     store_types: Vec<impl MakeKeyValueStore>,
-//     labels: Vec<&str>,
-// ) -> anyhow::Result<TestFactorsInstanceState> {
-//     let mut test_resolver = RuntimeConfigResolver::new();
-//     for store_type in store_types {
-//         test_resolver.register_store_type(store_type)?;
-//     }
-//     let test_resolver = Arc::new(test_resolver);
-//     let factors = TestFactors {
-//         key_value: KeyValueFactor::new(),
-//     };
-//     let labels_clone = labels.clone();
-//     let env = TestEnvironment::new(factors)
-//         .extend_manifest(toml! {
-//             [component.test-component]
-//             source = "does-not-exist.wasm"
-//             key_value_stores = labels_clone
-//         })
-//         .runtime_config(TomlConfig::new(test_resolver, runtime_config))?;
-//     let state = env.build_instance_state().await?;
-//     assert_eq!(
-//         labels,
-//         state.key_value.allowed_stores().iter().collect::<Vec<_>>()
-//     );
-
-//     Ok(state)
-// }
-
-// #[tokio::test]
-// async fn overridden_default_key_value_works() -> anyhow::Result<()> {
-//     let runtime_config = toml::toml! {
-//         [key_value_store.default]
-//         type = "redis"
-//         url = "redis://localhost:6379"
-//     };
-//     run_test_with_config_and_stores_for_label(
-//         Some(runtime_config),
-//         vec![RedisKeyValueStore::new()],
-//         vec!["default"],
-//     )
-//     .await?;
-//     Ok(())
-// }
-
-// #[tokio::test]
-// async fn custom_spin_key_value_works() -> anyhow::Result<()> {
-//     let runtime_config = toml::toml! {
-//         [key_value_store.custom]
-//         type = "spin"
-//     };
-//     run_test_with_config_and_stores_for_label(
-//         Some(runtime_config),
-//         vec![SpinKeyValueStore::new(None)],
-//         vec!["custom"],
-//     )
-//     .await?;
-//     Ok(())
-// }
-
-// #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
-// async fn custom_spin_key_value_works_with_absolute_path() -> anyhow::Result<()> {
-//     let tmp_dir = tempfile::TempDir::with_prefix("example")?;
-//     let db_path = tmp_dir.path().join("foo/custom.db");
-//     // Check that the db does not exist yet - it will exist by the end of the test
-//     assert!(!db_path.exists());
-
-//     let path_str = db_path.to_str().unwrap();
-//     let runtime_config = toml::toml! {
-//         [key_value_store.custom]
-//         type = "spin"
-//         path = path_str
-//     };
-//     let mut state = run_test_with_config_and_stores_for_label(
-//         Some(runtime_config),
-//         vec![SpinKeyValueStore::new(Some(
-//             std::env::current_dir().context("failed to get current directory")?,
-//         ))],
-//         vec!["custom"],
-//     )
-//     .await?;
-
-//     // Actually et a key since store creation is lazy
-//     let store = state.key_value.open("custom".to_owned()).await??;
-//     let _ = state.key_value.get(store, "foo".to_owned()).await??;
-
-//     // Check that the parent has been created
-//     assert!(db_path.exists());
-//     Ok(())
-// }
-
-// #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
-// async fn custom_spin_key_value_works_with_relative_path() -> anyhow::Result<()> {
-//     let tmp_dir = tempfile::TempDir::with_prefix("example")?;
-//     let db_path = tmp_dir.path().join("custom.db");
-//     // Check that the db does not exist yet - it will exist by the end of the test
-//     assert!(!db_path.exists());
-
-//     let runtime_config = toml::toml! {
-//         [key_value_store.custom]
-//         type = "spin"
-//         path = "custom.db"
-//     };
-//     let mut state = run_test_with_config_and_stores_for_label(
-//         Some(runtime_config),
-//         vec![SpinKeyValueStore::new(Some(tmp_dir.path().to_owned()))],
-//         vec!["custom"],
-//     )
-//     .await?;
-
-//     // Actually et a key since store creation is lazy
-//     let store = state.key_value.open("custom".to_owned()).await??;
-//     let _ = state.key_value.get(store, "foo".to_owned()).await??;
-
-//     // Check that the correct store in the config was chosen by verifying the existence of the DB
-//     assert!(db_path.exists());
-//     Ok(())
-// }
-
-// #[tokio::test]
-// async fn custom_redis_key_value_works() -> anyhow::Result<()> {
-//     let runtime_config = toml::toml! {
-//         [key_value_store.custom]
-//         type = "redis"
-//         url = "redis://localhost:6379"
-//     };
-//     run_test_with_config_and_stores_for_label(
-//         Some(runtime_config),
-//         vec![RedisKeyValueStore::new()],
-//         vec!["custom"],
-//     )
-//     .await?;
-//     Ok(())
-// }
-
-// #[tokio::test]
-// async fn misconfigured_spin_key_value_fails() -> anyhow::Result<()> {
-//     let tmp_dir = tempfile::TempDir::with_prefix("example")?;
-//     let runtime_config = toml::toml! {
-//         [key_value_store.custom]
-//         type = "spin"
-//         path = "/$$&/bad/path/foo.db"
-//     };
-//     let result = run_test_with_config_and_stores_for_label(
-//         Some(runtime_config),
-//         vec![SpinKeyValueStore::new(Some(tmp_dir.path().to_owned()))],
-//         vec!["custom"],
-//     )
-//     .await;
-//     // TODO(rylev): This only fails on my machine due to a read-only file system error.
-//     // We should consider adding a check for the error message.
-//     assert!(result.is_err());
-//     Ok(())
-// }

crates/runtime-config/src/lib.rs

@@ -300,7 +300,7 @@ impl FactorRuntimeConfigSource<KeyValueFactor> for TomlRuntimeConfigSource<'_, '
     fn get_runtime_config(
         &mut self,
     ) -> anyhow::Result<Option<spin_factor_key_value::RuntimeConfig>> {
-        self.key_value.resolve_from_toml(Some(&self.toml.table))
+        Ok(Some(self.key_value.resolve(Some(&self.toml.table))?))
     }
 }
 
@@ -368,7 +368,7 @@ impl FactorRuntimeConfigSource<OutboundMqttFactor> for TomlRuntimeConfigSource<'
 
 impl FactorRuntimeConfigSource<SqliteFactor> for TomlRuntimeConfigSource<'_, '_> {
     fn get_runtime_config(&mut self) -> anyhow::Result<Option<spin_factor_sqlite::RuntimeConfig>> {
-        self.sqlite.resolve_from_toml(&self.toml.table)
+        Ok(Some(self.sqlite.resolve(&self.toml.table)?))
     }
 }
 
@@ -438,32 +438,53 @@ fn sqlite_config_resolver(
 
 #[cfg(test)]
 mod tests {
+    use std::{collections::HashMap, sync::Arc};
+
     use spin_factors::RuntimeFactors;
 
     use super::*;
 
+    /// Define a test factor with the given field and factor type.
+    macro_rules! define_test_factor {
+        ($field:ident : $factor:ty) => {
+            #[derive(RuntimeFactors)]
+            struct TestFactors {
+                $field: $factor,
+            }
+            impl TryFrom<TomlRuntimeConfigSource<'_, '_>> for TestFactorsRuntimeConfig {
+                type Error = anyhow::Error;
+
+                fn try_from(value: TomlRuntimeConfigSource<'_, '_>) -> Result<Self, Self::Error> {
+                    Self::from_source(value)
+                }
+            }
+            fn resolve_toml(toml: toml::Table) -> ResolvedRuntimeConfig<TestFactorsRuntimeConfig> {
+                ResolvedRuntimeConfig::<TestFactorsRuntimeConfig>::new(
+                    toml_resolver(&toml),
+                    None,
+                    false,
+                )
+                .unwrap()
+            }
+        };
+    }
+
     #[test]
     fn sqlite_is_configured_correctly() {
-        #[derive(RuntimeFactors)]
-        struct Factors {
-            sqlite: SqliteFactor,
-        }
-        impl TryFrom<TomlRuntimeConfigSource<'_, '_>> for FactorsRuntimeConfig {
-            type Error = anyhow::Error;
-
-            fn try_from(value: TomlRuntimeConfigSource<'_, '_>) -> Result<Self, Self::Error> {
-                Self::from_source(value)
+        define_test_factor!(sqlite: SqliteFactor);
+
+        impl TestFactorsRuntimeConfig {
+            /// Get the connection creators for the configured sqlite databases.
+            fn connection_creators(
+                &self,
+            ) -> &HashMap<String, Arc<dyn spin_factor_sqlite::ConnectionCreator>> {
+                &self.sqlite.as_ref().unwrap().connection_creators
             }
-        }
 
-        impl FactorsRuntimeConfig {
             /// Get the labels of the configured sqlite databases.
             fn configured_labels(&self) -> Vec<&str> {
                 let mut configured_labels = self
-                    .sqlite
-                    .as_ref()
-                    .unwrap()
-                    .connection_creators
+                    .connection_creators()
                     .keys()
                     .map(|s| s.as_str())
                     .collect::<Vec<_>>();
@@ -478,20 +499,36 @@ mod tests {
             [sqlite_database.foo]
             type = "spin"
         };
-        let config =
-            ResolvedRuntimeConfig::<FactorsRuntimeConfig>::new(toml_resolver(&toml), None, false)
-                .unwrap();
         assert_eq!(
-            config.runtime_config.configured_labels(),
+            resolve_toml(toml).runtime_config.configured_labels(),
             vec!["default", "foo"]
         );
 
         // Test that the default label is added with an empty toml config.
         let toml = toml::Table::new();
-        let config =
-            ResolvedRuntimeConfig::<FactorsRuntimeConfig>::new(toml_resolver(&toml), None, false)
-                .unwrap();
-        assert_eq!(config.runtime_config.configured_labels(), vec!["default"]);
+        let runtime_config = resolve_toml(toml).runtime_config;
+        assert_eq!(runtime_config.configured_labels(), vec!["default"]);
+    }
+
+    #[test]
+    fn key_value_is_configured_correctly() {
+        define_test_factor!(key_value: KeyValueFactor);
+        impl TestFactorsRuntimeConfig {
+            /// Get whether the store manager exists for the given label.
+            fn has_store_manager(&self, label: &str) -> bool {
+                self.key_value.as_ref().unwrap().has_store_manager(label)
+            }
+        }
+
+        // Test that the default label is added if not provided.
+        let toml = toml::toml! {
+            [key_value_store.foo]
+            type = "spin"
+        };
+        let runtime_config = resolve_toml(toml).runtime_config;
+        assert!(["default", "foo"]
+            .iter()
+            .all(|label| { runtime_config.has_store_manager(label) }));
     }
 
     fn toml_resolver(toml: &toml::Table) -> TomlResolver<'_> {

crates/sqlite/src/lib.rs

@@ -52,7 +52,23 @@ impl RuntimeConfigResolver {
     /// ```
     ///
     /// Configuration is automatically added for the 'default' label if it is not provided.
-    pub fn resolve_from_toml(
+    pub fn resolve(
+        &self,
+        table: &impl GetTomlValue,
+    ) -> anyhow::Result<spin_factor_sqlite::runtime_config::RuntimeConfig> {
+        let mut runtime_config = self.resolve_from_toml(table)?.unwrap_or_default();
+        // If the user did not provide configuration for the default label, add it.
+        if !runtime_config.connection_creators.contains_key("default") {
+            runtime_config
+                .connection_creators
+                .insert("default".to_owned(), self.default());
+        }
+
+        Ok(runtime_config)
+    }
+
+    /// Get the runtime configuration for SQLite databases from a TOML table.
+    fn resolve_from_toml(
         &self,
         table: &impl GetTomlValue,
     ) -> anyhow::Result<Option<spin_factor_sqlite::runtime_config::RuntimeConfig>> {
@@ -61,16 +77,11 @@ impl RuntimeConfigResolver {
         };
         let config: std::collections::HashMap<String, TomlRuntimeConfig> =
             table.clone().try_into()?;
-        let mut connection_creators = config
+        let connection_creators = config
             .into_iter()
             .map(|(k, v)| Ok((k, self.get_connection_creator(v)?)))
             .collect::<anyhow::Result<HashMap<_, _>>>()?;
 
-        // If the user did not provide configuration for the default label, add it.
-        if connection_creators.contains_key("default") {
-            connection_creators.insert("default".to_owned(), self.default());
-        }
-
         Ok(Some(spin_factor_sqlite::runtime_config::RuntimeConfig {
             connection_creators,
         }))