Merge pull request #560 from imageworks/296-test-trait

Move test definition behind a trait

Author: rydrman

crates/spk-cli/cmd-test/src/cmd_test.rs

@@ -4,13 +4,14 @@
 use std::str::FromStr;
 use std::sync::Arc;
 
-use anyhow::Result;
+use anyhow::{Context, Result};
 use clap::Args;
 use spk_build::BuildSource;
 use spk_cli_common::{flags, CommandArgs, Run};
 use spk_schema::foundation::format::{FormatIdent, FormatOptionMap};
 use spk_schema::foundation::ident_build::Build;
 use spk_schema::foundation::option_map::{host_options, OptionMap};
+use spk_schema::prelude::*;
 use spk_schema::{Recipe, TestStage, Variant};
 
 use crate::test::{PackageBuildTester, PackageInstallTester, PackageSourceTester, Tester};
@@ -23,7 +24,7 @@ mod cmd_test_test;
 ///
 /// In order to run install tests the package must have been built already
 #[derive(Args)]
-pub struct Test {
+pub struct CmdTest {
     #[clap(flatten)]
     pub options: flags::Options,
     #[clap(flatten)]
@@ -57,7 +58,7 @@ pub struct Test {
 }
 
 #[async_trait::async_trait]
-impl Run for Test {
+impl Run for CmdTest {
     async fn run(&mut self) -> Result<i32> {
         let options = self.options.get_options()?;
         let (_runtime, repos) = tokio::try_join!(
@@ -123,11 +124,15 @@ impl Run for Test {
                         continue;
                     }
 
-                    for (index, test) in recipe.get_tests(&opts)?.into_iter().enumerate() {
-                        if test.stage != stage {
-                            continue;
-                        }
-
+                    let selected = recipe
+                        .get_tests(stage, &opts)
+                        .context("Failed to select tests for this variant")?;
+                    tracing::info!(
+                        variant=%opts.format_option_map(),
+                        "Running {} relevant tests for this variant",
+                        selected.len()
+                    );
+                    for (index, test) in selected.into_iter().enumerate() {
                         let mut builder = self
                             .formatter_settings
                             .get_formatter_builder(self.verbose)?;
@@ -140,31 +145,27 @@ impl Run for Test {
 
                         let mut tester: Box<dyn Tester> = match stage {
                             TestStage::Sources => {
-                                let mut tester = PackageSourceTester::new(
-                                    (*recipe).clone(),
-                                    test.script.join("\n"),
-                                );
+                                let mut tester =
+                                    PackageSourceTester::new((*recipe).clone(), test.script());
 
                                 tester
                                     .with_options(opts.clone())
                                     .with_repositories(repos.iter().cloned())
-                                    .with_requirements(test.requirements.clone())
+                                    .with_requirements(test.additional_requirements())
                                     .with_source(source.clone())
                                     .watch_environment_resolve(&src_formatter);
 
                                 Box::new(tester)
                             }
 
                             TestStage::Build => {
-                                let mut tester = PackageBuildTester::new(
-                                    (*recipe).clone(),
-                                    test.script.join("\n"),
-                                );
+                                let mut tester =
+                                    PackageBuildTester::new((*recipe).clone(), test.script());
 
                                 tester
                                     .with_options(opts.clone())
                                     .with_repositories(repos.iter().cloned())
-                                    .with_requirements(test.requirements.clone())
+                                    .with_requirements(test.additional_requirements())
                                     .with_source(
                                         source.clone().map(BuildSource::LocalPath).unwrap_or_else(
                                             || {
@@ -186,40 +187,24 @@ impl Run for Test {
                             TestStage::Install => {
                                 let mut tester = PackageInstallTester::new(
                                     (*recipe).clone(),
-                                    test.script.join("\n"),
+                                    test.script(),
                                     variant,
                                 );
 
                                 tester
                                     .with_options(opts.clone())
                                     .with_repositories(repos.iter().cloned())
-                                    .with_requirements(test.requirements.clone())
+                                    .with_requirements(test.additional_requirements())
                                     .with_source(source.clone())
                                     .watch_environment_resolve(&install_formatter);
 
                                 Box::new(tester)
                             }
                         };
 
-                        let mut selected = false;
-                        for selector in test.selectors.iter() {
-                            let mut selected_opts = opts.clone();
-                            selected_opts.extend(selector.clone());
-                            if selected_opts.digest() == digest {
-                                selected = true;
-                            }
-                        }
-                        if !selected && !test.selectors.is_empty() {
-                            tracing::info!(
-                                "SKIP #{index}: variant not selected: {}",
-                                opts.format_option_map()
-                            );
-                            continue;
-                        }
-
                         tracing::info!(
-                            "Running test #{index} variant={}",
-                            opts.format_option_map()
+                            variant=%opts.format_option_map(),
+                            "Running selected test #{index}",
                         );
 
                         tester.test().await?
@@ -231,7 +216,7 @@ impl Run for Test {
     }
 }
 
-impl CommandArgs for Test {
+impl CommandArgs for CmdTest {
     fn get_positional_args(&self) -> Vec<String> {
         // The important positional args for a test are the packages
         self.packages.clone()

crates/spk-cli/cmd-test/src/cmd_test_test.rs

@@ -12,7 +12,7 @@ use spk_cmd_build::cmd_build::Build;
 use spk_schema::foundation::fixtures::*;
 use spk_storage::fixtures::*;
 
-use super::Test;
+use super::CmdTest;
 
 #[derive(Parser)]
 struct BuildOpt {
@@ -23,7 +23,7 @@ struct BuildOpt {
 #[derive(Parser)]
 struct TestOpt {
     #[clap(flatten)]
-    test: Test,
+    test: CmdTest,
 }
 
 macro_rules! build_package {

crates/spk-schema/src/lib.rs

@@ -22,7 +22,7 @@ mod requirements_list;
 mod source_spec;
 mod spec;
 mod template;
-mod test_spec;
+mod test;
 pub mod v0;
 mod validation;
 pub mod variant;
@@ -57,7 +57,7 @@ pub use spk_schema_foundation::{
 };
 pub use spk_schema_ident::{self as ident, AnyIdent, BuildIdent, Request, VersionIdent};
 pub use template::{Template, TemplateData, TemplateExt};
-pub use test_spec::TestStage;
+pub use test::{Test, TestStage};
 pub use validation::{default_validators, ValidationSpec, Validator};
 pub use variant::{Variant, VariantExt};
 pub use {serde_json, spk_schema_validators as validators};

crates/spk-schema/src/prelude.rs

@@ -11,6 +11,7 @@ pub use super::{
     Package,
     Recipe,
     Template,
+    Test,
     Variant,
     VariantExt,
 };

crates/spk-schema/src/recipe.rs

@@ -10,8 +10,7 @@ use spk_schema_ident::VersionIdent;
 use crate::foundation::option_map::OptionMap;
 use crate::foundation::spec_ops::{Named, Versioned};
 use crate::ident::Request;
-use crate::test_spec::TestSpec;
-use crate::{Package, Result, Variant};
+use crate::{Package, Result, TestStage, Variant};
 
 /// Return the resolved packages from a solution.
 pub trait BuildEnv {
@@ -27,6 +26,7 @@ pub trait Recipe:
 {
     type Output: super::Package;
     type Variant: super::Variant + Clone;
+    type Test: super::Test;
 
     /// Build an identifier to represent this recipe.
     ///
@@ -53,8 +53,8 @@ pub trait Recipe:
     where
         V: Variant;
 
-    /// Return the tests defined for this package.
-    fn get_tests<V>(&self, variant: &V) -> Result<Vec<TestSpec>>
+    /// Return the tests defined for this package at the given stage.
+    fn get_tests<V>(&self, stage: TestStage, variant: &V) -> Result<Vec<Self::Test>>
     where
         V: Variant;
 
@@ -75,6 +75,7 @@ where
 {
     type Output = T::Output;
     type Variant = T::Variant;
+    type Test = T::Test;
 
     fn ident(&self) -> &VersionIdent {
         (**self).ident()
@@ -98,11 +99,11 @@ where
         (**self).get_build_requirements(variant)
     }
 
-    fn get_tests<V>(&self, variant: &V) -> Result<Vec<TestSpec>>
+    fn get_tests<V>(&self, stage: TestStage, variant: &V) -> Result<Vec<Self::Test>>
     where
         V: Variant,
     {
-        (**self).get_tests(variant)
+        (**self).get_tests(stage, variant)
     }
 
     fn generate_source_build(&self, root: &Path) -> Result<Self::Output> {
@@ -125,6 +126,7 @@ where
 {
     type Output = T::Output;
     type Variant = T::Variant;
+    type Test = T::Test;
 
     fn ident(&self) -> &VersionIdent {
         (**self).ident()
@@ -148,11 +150,11 @@ where
         (**self).get_build_requirements(variant)
     }
 
-    fn get_tests<V>(&self, variant: &V) -> Result<Vec<TestSpec>>
+    fn get_tests<V>(&self, stage: TestStage, variant: &V) -> Result<Vec<Self::Test>>
     where
         V: Variant,
     {
-        (**self).get_tests(variant)
+        (**self).get_tests(stage, variant)
     }
 
     fn generate_source_build(&self, root: &Path) -> Result<Self::Output> {

crates/spk-schema/src/spec.rs

@@ -19,7 +19,6 @@ use crate::foundation::option_map::OptionMap;
 use crate::foundation::spec_ops::prelude::*;
 use crate::foundation::version::{Compat, Compatibility, Version};
 use crate::ident::{PkgRequest, Request, Satisfy, VarRequest};
-use crate::test_spec::TestSpec;
 use crate::{
     v0,
     BuildEnv,
@@ -34,6 +33,8 @@ use crate::{
     Result,
     Template,
     TemplateExt,
+    Test,
+    TestStage,
     Variant,
 };
 
@@ -231,6 +232,7 @@ pub enum SpecRecipe {
 impl Recipe for SpecRecipe {
     type Output = Spec;
     type Variant = SpecVariant;
+    type Test = SpecTest;
 
     fn ident(&self) -> &VersionIdent {
         match self {
@@ -271,12 +273,16 @@ impl Recipe for SpecRecipe {
         }
     }
 
-    fn get_tests<V>(&self, variant: &V) -> Result<Vec<TestSpec>>
+    fn get_tests<V>(&self, stage: TestStage, variant: &V) -> Result<Vec<Self::Test>>
     where
         V: Variant,
     {
         match self {
-            SpecRecipe::V0Package(r) => r.get_tests(variant),
+            SpecRecipe::V0Package(r) => Ok(r
+                .get_tests(stage, variant)?
+                .into_iter()
+                .map(SpecTest::V0)
+                .collect()),
         }
     }
 
@@ -395,6 +401,24 @@ impl std::fmt::Display for SpecVariant {
     }
 }
 
+pub enum SpecTest {
+    V0(v0::TestSpec),
+}
+
+impl Test for SpecTest {
+    fn script(&self) -> String {
+        match self {
+            Self::V0(t) => t.script(),
+        }
+    }
+
+    fn additional_requirements(&self) -> Vec<Request> {
+        match self {
+            Self::V0(t) => t.additional_requirements(),
+        }
+    }
+}
+
 /// Specifies some data object within the spk ecosystem.
 ///
 /// All resolve-able types have a spec representation that can be serialized