Some refactorings

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

Author: rylev

Cargo.lock

@@ -12,5 +12,4 @@ rust-version.workspace = true
 anyhow = "1.0"
 env_logger = "0.10.0"
 log = "0.4"
-reqwest = { workspace = true }
 testing-framework = { path = "../testing-framework" }

tests/integration.rs

@@ -1,8 +1,13 @@
 use anyhow::Context;
 use std::path::{Path, PathBuf};
 use testing_framework::{
-    EnvTemplate, InMemorySpin, OnTestError, Runtime, ServicesConfig, Spin, SpinConfig,
-    TestEnvironment, TestEnvironmentConfig, TestError, TestResult,
+    http::{Method, Request},
+    runtimes::{
+        in_process_spin::InProcessSpin,
+        spin_cli::{SpinCli, SpinConfig},
+    },
+    EnvTemplate, OnTestError, Runtime, ServicesConfig, TestEnvironment, TestEnvironmentConfig,
+    TestError, TestResult,
 };
 
 /// Configuration for a runtime test
@@ -19,45 +24,34 @@ pub struct RuntimeTest<R> {
     env: TestEnvironment<R>,
 }
 
-impl RuntimeTest<Spin> {
+impl RuntimeTest<SpinCli> {
     /// Run the runtime tests suite.
     ///
     /// Error represents an error in bootstrapping the tests. What happens on individual test failures
     /// is controlled by `on_error`.
     pub fn run_all(
-        tests_path: &Path,
+        tests_dir_path: &Path,
         spin_binary: PathBuf,
         on_error: OnTestError,
     ) -> anyhow::Result<()> {
-        for test in std::fs::read_dir(tests_path)
-            .with_context(|| format!("failed to read test directory '{}'", tests_path.display()))?
-        {
-            let test = test.context("I/O error reading entry from test directory")?;
-            if !test.file_type()?.is_dir() {
-                log::debug!(
-                    "Ignoring non-sub-directory in test directory: {}",
-                    test.path().display()
-                );
-                continue;
-            }
-
+        Self::run_on_all(tests_dir_path, |test_path| {
             let config = RuntimeTestConfig {
-                test_path: test.path(),
+                test_path,
                 runtime_config: SpinConfig {
                     binary_path: spin_binary.clone(),
                 },
                 on_error,
             };
             Self::bootstrap(config)?.run();
-        }
-        Ok(())
+            Ok(())
+        })
     }
 
-    pub fn bootstrap(config: RuntimeTestConfig<Spin>) -> anyhow::Result<Self> {
+    pub fn bootstrap(config: RuntimeTestConfig<SpinCli>) -> anyhow::Result<Self> {
         log::info!("Testing: {}", config.test_path.display());
         let test_path_clone = config.test_path.to_owned();
         let spin_binary = config.runtime_config.binary_path.clone();
-        let preboot = move |env: &mut TestEnvironment<Spin>| {
+        let preboot = move |env: &mut TestEnvironment<SpinCli>| {
             copy_manifest(&test_path_clone, env)?;
             Ok(())
         };
@@ -67,7 +61,7 @@ impl RuntimeTest<Spin> {
             [],
             preboot,
             services_config,
-            testing_framework::SpinMode::Http,
+            testing_framework::runtimes::SpinAppType::Http,
         );
         let env = TestEnvironment::up(env_config)?;
         Ok(Self {
@@ -81,7 +75,7 @@ impl RuntimeTest<Spin> {
     pub fn run(&mut self) {
         self.run_test(|env| {
             let runtime = env.runtime_mut();
-            let request = testing_framework::Request::new(reqwest::Method::GET, "/");
+            let request = Request::new(Method::GET, "/");
             let response = runtime.make_http_request(request)?;
             if response.status() == 200 {
                 return Ok(());
@@ -99,44 +93,33 @@ impl RuntimeTest<Spin> {
 
             Err(TestError::Failure(RuntimeTestFailure {
                 error: text,
-                stderr: runtime.stderr().to_owned(),
+                stderr: Some(runtime.stderr().to_owned()),
             }))
         })
     }
 }
 
-impl RuntimeTest<InMemorySpin> {
+impl RuntimeTest<InProcessSpin> {
     /// Run the runtime tests suite.
     ///
     /// Error represents an error in bootstrapping the tests. What happens on individual test failures
     /// is controlled by `on_error`.
-    pub fn run_all(tests_path: &Path, on_error: OnTestError) -> anyhow::Result<()> {
-        for test in std::fs::read_dir(tests_path)
-            .with_context(|| format!("failed to read test directory '{}'", tests_path.display()))?
-        {
-            let test = test.context("I/O error reading entry from test directory")?;
-            if !test.file_type()?.is_dir() {
-                log::debug!(
-                    "Ignoring non-sub-directory in test directory: {}",
-                    test.path().display()
-                );
-                continue;
-            }
-
+    pub fn run_all(tests_dir_path: &Path, on_error: OnTestError) -> anyhow::Result<()> {
+        Self::run_on_all(tests_dir_path, |test_path| {
             let config = RuntimeTestConfig {
-                test_path: test.path(),
+                test_path,
                 runtime_config: (),
                 on_error,
             };
             Self::bootstrap(config)?.run();
-        }
-        Ok(())
+            Ok(())
+        })
     }
 
-    pub fn bootstrap(config: RuntimeTestConfig<InMemorySpin>) -> anyhow::Result<Self> {
+    pub fn bootstrap(config: RuntimeTestConfig<InProcessSpin>) -> anyhow::Result<Self> {
         log::info!("Testing: {}", config.test_path.display());
         let test_path_clone = config.test_path.to_owned();
-        let preboot = move |env: &mut TestEnvironment<InMemorySpin>| {
+        let preboot = move |env: &mut TestEnvironment<InProcessSpin>| {
             copy_manifest(&test_path_clone, env)?;
             Ok(())
         };
@@ -153,7 +136,7 @@ impl RuntimeTest<InMemorySpin> {
     pub fn run(&mut self) {
         self.run_test(|env| {
             let runtime = env.runtime_mut();
-            let response = runtime.make_http_request(testing_framework::Request::new(reqwest::Method::GET, "/"))?;
+            let response = runtime.make_http_request(Request::new(Method::GET, "/"))?;
             if response.status() == 200 {
                 return Ok(());
             }
@@ -169,13 +152,37 @@ impl RuntimeTest<InMemorySpin> {
 
             Err(TestError::Failure(RuntimeTestFailure {
                 error: text,
-                stderr: String::new()
+                stderr: None
             }))
         })
     }
 }
 
 impl<R> RuntimeTest<R> {
+    /// Run a closure against all tests in the given tests directory
+    pub fn run_on_all(
+        tests_dir_path: &Path,
+        run: impl Fn(PathBuf) -> anyhow::Result<()>,
+    ) -> anyhow::Result<()> {
+        for test in std::fs::read_dir(tests_dir_path).with_context(|| {
+            format!(
+                "failed to read test directory '{}'",
+                tests_dir_path.display()
+            )
+        })? {
+            let test = test.context("I/O error reading entry from test directory")?;
+            if !test.file_type()?.is_dir() {
+                log::debug!(
+                    "Ignoring non-sub-directory in test directory: {}",
+                    test.path().display()
+                );
+                continue;
+            }
+            run(test.path())?;
+        }
+        Ok(())
+    }
+
     /// Run an individual test
     pub(crate) fn run_test(
         &mut self,
@@ -220,15 +227,21 @@ impl<R> RuntimeTest<R> {
                 } else {
                     error!(
                     on_error,
-                    "Test errored but not in the expected way.\n\texpected: {expected}\n\tgot: {error}\n\nstderr:\n{stderr}",
+                    "Test errored but not in the expected way.\n\texpected: {expected}\n\tgot: {error}{}",
+                    stderr
+                        .map(|e| format!("\n\nstderr:\n{e}"))
+                        .unwrap_or_default()
                 )
                 }
             }
             Err(TestError::Failure(RuntimeTestFailure { error, stderr })) => {
                 error!(
                     on_error,
-                    "Test '{}' errored: {error}\nstderr:\n{stderr}",
-                    self.test_path.display()
+                    "Test '{}' errored: {error}{}",
+                    self.test_path.display(),
+                    stderr
+                        .map(|e| format!("\nstderr:\n{e}"))
+                        .unwrap_or_default()
                 );
             }
             Err(TestError::Fatal(extra)) => {
@@ -287,6 +300,6 @@ fn copy_manifest<R>(test_dir: &Path, env: &mut TestEnvironment<R>) -> anyhow::Re
 struct RuntimeTestFailure {
     /// The error message returned by the runtime
     error: String,
-    /// The runtime's stderr
-    stderr: String,
+    /// The runtime's stderr if there is one
+    stderr: Option<String>,
 }

tests/runtime-tests/Cargo.toml

@@ -1,7 +1,8 @@
 use std::path::PathBuf;
 
 use runtime_tests::RuntimeTest;
-use testing_framework::{OnTestError, Spin};
+use testing_framework::runtimes::spin_cli::SpinCli;
+use testing_framework::OnTestError;
 
 fn main() -> anyhow::Result<()> {
     env_logger::init();
@@ -12,5 +13,5 @@ fn main() -> anyhow::Result<()> {
         .unwrap_or_else(|| PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests"));
 
     let config = OnTestError::Log;
-    RuntimeTest::<Spin>::run_all(&tests_path, spin_binary_path, config)
+    RuntimeTest::<SpinCli>::run_all(&tests_path, spin_binary_path, config)
 }

tests/runtime-tests/src/lib.rs

@@ -2,6 +2,8 @@
 mod runtime_tests {
     use std::path::PathBuf;
 
+    use testing_framework::runtimes::in_process_spin::InProcessSpin;
+
     // The macro inspects the tests directory and
     // generates individual tests for each one.
     test_codegen_macro::codegen_runtime_tests!(
@@ -16,12 +18,9 @@ mod runtime_tests {
         let config = runtime_tests::RuntimeTestConfig {
             test_path,
             runtime_config: (),
-            // runtime_config: testing_framework::SpinConfig {
-            //     binary_path: env!("CARGO_BIN_EXE_spin").into(),
-            // },
             on_error: testing_framework::OnTestError::Panic,
         };
-        runtime_tests::RuntimeTest::<testing_framework::InMemorySpin>::bootstrap(config)
+        runtime_tests::RuntimeTest::<InProcessSpin>::bootstrap(config)
             .expect("failed to bootstrap runtime tests tests")
             .run();
     }

tests/runtime-tests/src/main.rs

@@ -4,15 +4,15 @@ use std::path::PathBuf;
 /// Run an integration test
 pub fn run_test(
     test_name: impl Into<String>,
-    mode: testing_framework::SpinMode,
+    app_type: testing_framework::runtimes::SpinAppType,
     spin_up_args: impl IntoIterator<Item = String>,
     services_config: testing_framework::ServicesConfig,
     test: impl FnOnce(
-            &mut testing_framework::TestEnvironment<testing_framework::Spin>,
+            &mut testing_framework::TestEnvironment<testing_framework::runtimes::spin_cli::SpinCli>,
         ) -> testing_framework::TestResult<anyhow::Error>
         + 'static,
 ) -> testing_framework::TestResult<anyhow::Error> {
-    let mut env = bootstap_env(test_name, spin_up_args, services_config, mode)
+    let mut env = bootstap_env(test_name, spin_up_args, services_config, app_type)
         .context("failed to boot test environment")?;
     test(&mut env)?;
     Ok(())
@@ -23,24 +23,26 @@ pub fn bootstap_env(
     test_name: impl Into<String>,
     spin_up_args: impl IntoIterator<Item = String>,
     services_config: testing_framework::ServicesConfig,
-    mode: testing_framework::SpinMode,
-) -> anyhow::Result<testing_framework::TestEnvironment<testing_framework::Spin>> {
+    app_type: testing_framework::runtimes::SpinAppType,
+) -> anyhow::Result<
+    testing_framework::TestEnvironment<testing_framework::runtimes::spin_cli::SpinCli>,
+> {
     let test_name = test_name.into();
     let config = testing_framework::TestEnvironmentConfig::spin(
         spin_binary(),
         spin_up_args,
         move |env| preboot(&test_name, env),
         services_config,
-        mode,
+        app_type,
     );
     testing_framework::TestEnvironment::up(config)
 }
 
 /// Assert that a request to the spin server returns the expected status and body
 pub fn assert_spin_request<B: Into<reqwest::Body>>(
-    spin: &mut testing_framework::Spin,
-    request: testing_framework::Request<'_, B>,
-    expected: testing_framework::Response,
+    spin: &mut testing_framework::runtimes::spin_cli::SpinCli,
+    request: testing_framework::http::Request<'_, B>,
+    expected: testing_framework::http::Response,
 ) -> testing_framework::TestResult<anyhow::Error> {
     let uri = request.uri;
     let r = spin.make_http_request(request)?;
@@ -97,7 +99,7 @@ impl<'a, T: std::fmt::Display> std::fmt::Display for TruncatedSlice<'a, T> {
 /// Get the test environment ready to run a test
 fn preboot(
     test: &str,
-    env: &mut testing_framework::TestEnvironment<testing_framework::Spin>,
+    env: &mut testing_framework::TestEnvironment<testing_framework::runtimes::spin_cli::SpinCli>,
 ) -> anyhow::Result<()> {
     let test_path = format!("tests/testcases/{test}");
     for file in std::fs::read_dir(test_path)? {
@@ -159,13 +161,13 @@ pub fn http_smoke_test_template_with_route(
         |_| Ok(Vec::new()),
         prebuild_hook,
         |_| Ok(Vec::new()),
-        testing_framework::SpinMode::Http,
+        testing_framework::runtimes::SpinAppType::Http,
     )?;
 
     assert_spin_request(
         env.runtime_mut(),
-        testing_framework::Request::new(reqwest::Method::GET, route),
-        testing_framework::Response::full(200, Default::default(), expected_body),
+        testing_framework::http::Request::new(testing_framework::http::Method::GET, route),
+        testing_framework::http::Response::full(200, Default::default(), expected_body),
     )?;
 
     Ok(())
@@ -195,7 +197,7 @@ pub fn redis_smoke_test_template(
         },
         prebuild_hook,
         |_| Ok(Vec::new()),
-        testing_framework::SpinMode::Redis,
+        testing_framework::runtimes::SpinAppType::Redis,
     )?;
     let redis_port = env
         .get_port(6379)?
@@ -234,8 +236,10 @@ pub fn bootstrap_smoke_test(
     spin_up_args: impl FnOnce(
         &mut testing_framework::TestEnvironment<()>,
     ) -> anyhow::Result<Vec<String>>,
-    spin_mode: testing_framework::SpinMode,
-) -> anyhow::Result<testing_framework::TestEnvironment<testing_framework::Spin>> {
+    spin_app_type: testing_framework::runtimes::SpinAppType,
+) -> anyhow::Result<
+    testing_framework::TestEnvironment<testing_framework::runtimes::spin_cli::SpinCli>,
+> {
     let mut env: testing_framework::TestEnvironment<()> =
         testing_framework::TestEnvironment::boot(services)?;
 
@@ -291,7 +295,12 @@ pub fn bootstrap_smoke_test(
     build.env("PATH", path).args(["build"]);
     env.run_in(&mut build)?;
     let spin_up_args = spin_up_args(&mut env)?;
-    let spin = testing_framework::Spin::start(&spin_binary(), &env, spin_up_args, spin_mode)?;
+    let spin = testing_framework::runtimes::spin_cli::SpinCli::start(
+        &spin_binary(),
+        &env,
+        spin_up_args,
+        spin_app_type,
+    )?;
     let env = env.start_runtime(spin)?;
     Ok(env)
 }