Skip to content

Commit

Permalink
Merge pull request #460 from imageworks/cmd-test-code-refactor
Browse files Browse the repository at this point in the history 
Reduce code duplication in cmd_test
  • Loading branch information
@jrray
jrray authored Aug 26, 2022
2 parents 7b9059a + 0c671b8 commit 60ccec5
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 165 deletions.
128 changes: 71 additions & 57 deletions crates/spk-cli/cmd-test/src/cmd_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use spk_schema::foundation::spec_ops::RecipeOps;
use spk_schema::ident::parse_ident;
use spk_schema::{Recipe, Template, TestStage};

use crate::test::{PackageBuildTester, PackageInstallTester, PackageSourceTester, Tester};

#[cfg(test)]
#[path = "./cmd_test_test.rs"]
mod cmd_test_test;
Expand Down Expand Up @@ -146,26 +148,6 @@ impl Run for Test {
continue;
}

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()
);

let mut builder =
self.formatter_settings.get_formatter_builder(self.verbose);
let src_formatter = builder.with_header("Source Resolver ").build();
Expand All @@ -175,58 +157,90 @@ impl Run for Test {
let install_formatter =
builder.with_header("Install Env Resolver ").build();

match stage {
let mut tester: Box<dyn Tester> = match stage {
TestStage::Sources => {
super::test::PackageSourceTester::new(
let mut tester = PackageSourceTester::new(
(*recipe).clone(),
test.script.join("\n"),
)
.with_options(opts.clone())
.with_repositories(repos.iter().cloned())
.with_requirements(test.requirements.clone())
.with_source(source.clone())
.watch_environment_resolve(&src_formatter)
.test()
.await?
);

tester
.with_options(opts.clone())
.with_repositories(repos.iter().cloned())
.with_requirements(test.requirements.clone())
.with_source(source.clone())
.watch_environment_resolve(&src_formatter);

Box::new(tester)
}

TestStage::Build => {
super::test::PackageBuildTester::new(
let mut tester = PackageBuildTester::new(
(*recipe).clone(),
test.script.join("\n"),
)
.with_options(opts.clone())
.with_repositories(repos.iter().cloned())
.with_requirements(test.requirements.clone())
.with_source(
source.clone().map(BuildSource::LocalPath).unwrap_or_else(
|| {
BuildSource::SourcePackage(
recipe.to_ident().into_build(Build::Source).into(),
)
},
),
)
.with_source_resolver(&build_src_formatter)
.with_build_resolver(&build_formatter)
.test()
.await?
);

tester
.with_options(opts.clone())
.with_repositories(repos.iter().cloned())
.with_requirements(test.requirements.clone())
.with_source(
source.clone().map(BuildSource::LocalPath).unwrap_or_else(
|| {
BuildSource::SourcePackage(
recipe
.to_ident()
.into_build(Build::Source)
.into(),
)
},
),
)
.with_source_resolver(&build_src_formatter)
.with_build_resolver(&build_formatter);

Box::new(tester)
}

TestStage::Install => {
super::test::PackageInstallTester::new(
let mut tester = PackageInstallTester::new(
(*recipe).clone(),
test.script.join("\n"),
)
.with_options(opts.clone())
.with_repositories(repos.iter().cloned())
.with_requirements(test.requirements.clone())
.with_source(source.clone())
.watch_environment_resolve(&install_formatter)
.test()
.await?
);

tester
.with_options(opts.clone())
.with_repositories(repos.iter().cloned())
.with_requirements(test.requirements.clone())
.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()
);

tester.test().await?
}
}
}
Expand Down
56 changes: 20 additions & 36 deletions crates/spk-cli/cmd-test/src/test/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/imageworks/spk

use std::io::Write;
use std::path::PathBuf;
use std::convert::TryInto;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::{convert::TryInto, ffi::OsString};

use spk_build::{source_package_path, BuildSource};
use spk_cli_common::{Error, Result, TestError};
use spk_cli_common::Result;
use spk_exec::resolve_runtime_layers;
use spk_schema::foundation::ident_build::Build;
use spk_schema::foundation::ident_component::Component;
Expand All @@ -21,6 +20,8 @@ use spk_solve::solution::Solution;
use spk_solve::{BoxedResolverCallback, DefaultResolver, ResolverCallback, Solver};
use spk_storage::{self as storage};

use super::Tester;

pub struct PackageBuildTester<'a> {
prefix: PathBuf,
recipe: SpecRecipe,
Expand Down Expand Up @@ -143,16 +144,7 @@ impl<'a> PackageBuildTester<'a> {
.recipe
.generate_binary_build(&self.options, &solution)?;

let mut env = solution.to_environment(Some(std::env::vars()));
env.insert(
"PREFIX".to_string(),
self.prefix
.to_str()
.ok_or_else(|| {
Error::String("Test prefix must be a valid unicode string".to_string())
})?
.to_string(),
);
let env = solution.to_environment(Some(std::env::vars()));

let source_dir = match &self.source {
BuildSource::SourcePackage(source) => {
Expand All @@ -161,28 +153,7 @@ impl<'a> PackageBuildTester<'a> {
BuildSource::LocalPath(path) => path.clone(),
};

let tmpdir = tempfile::Builder::new().prefix("spk-test").tempdir()?;
let script_path = tmpdir.path().join("test.sh");
let mut script_file = std::fs::File::create(&script_path)?;
script_file.write_all(self.script.as_bytes())?;
script_file.sync_data()?;
// TODO: this should be more easily configurable on the spfs side
std::env::set_var("SHELL", "bash");
let cmd = spfs::build_shell_initialized_command(
&rt,
OsString::from("bash"),
&[OsString::from("-ex"), script_path.into_os_string()],
)?;
let mut cmd = cmd.into_std();
let status = cmd.envs(env).current_dir(source_dir).status()?;
if !status.success() {
Err(TestError::new_error(format!(
"Test script returned non-zero exit status: {}",
status.code().unwrap_or(1)
)))
} else {
Ok(())
}
self.execute_test_script(&source_dir, env, &rt)
}

async fn resolve_source_package(&mut self, package: &Ident) -> Result<Solution> {
Expand Down Expand Up @@ -213,3 +184,16 @@ impl<'a> PackageBuildTester<'a> {
Ok(solution?)
}
}

#[async_trait::async_trait]
impl<'a> Tester for PackageBuildTester<'a> {
async fn test(&mut self) -> Result<()> {
self.test().await
}
fn prefix(&self) -> &Path {
&self.prefix
}
fn script(&self) -> &String {
&self.script
}
}
55 changes: 19 additions & 36 deletions crates/spk-cli/cmd-test/src/test/install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/imageworks/spk

use std::ffi::OsString;
use std::io::Write;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;

use spk_cli_common::{Error, Result, TestError};
use spk_cli_common::Result;
use spk_exec::resolve_runtime_layers;
use spk_schema::foundation::ident_component::Component;
use spk_schema::foundation::option_map::OptionMap;
Expand All @@ -18,6 +17,8 @@ use spk_solve::graph::Graph;
use spk_solve::{BoxedResolverCallback, DefaultResolver, ResolverCallback, Solver};
use spk_storage::{self as storage};

use super::Tester;

pub struct PackageInstallTester<'a> {
prefix: PathBuf,
recipe: SpecRecipe,
Expand Down Expand Up @@ -118,44 +119,26 @@ impl<'a> PackageInstallTester<'a> {
rt.save_state_to_storage().await?;
spfs::remount_runtime(&rt).await?;

let mut env = solution.to_environment(Some(std::env::vars()));
env.insert(
"PREFIX".to_string(),
self.prefix
.to_str()
.ok_or_else(|| {
Error::String("Test prefix must be a valid unicode string".to_string())
})?
.to_string(),
);
let env = solution.to_environment(Some(std::env::vars()));

let source_dir = match &self.source {
Some(source) => source.clone(),
None => PathBuf::from("."),
};

let tmpdir = tempfile::Builder::new().prefix("spk-test").tempdir()?;
let script_path = tmpdir.path().join("test.sh");
let mut script_file = std::fs::File::create(&script_path)?;
script_file.write_all(self.script.as_bytes())?;
script_file.sync_data()?;

// TODO: this should be more easily configurable on the spfs side
std::env::set_var("SHELL", "bash");
let cmd = spfs::build_shell_initialized_command(
&rt,
OsString::from("bash"),
&[OsString::from("-ex"), script_path.into_os_string()],
)?;
let mut cmd = cmd.into_std();
let status = cmd.envs(env).current_dir(source_dir).status()?;
if !status.success() {
Err(TestError::new_error(format!(
"Test script returned non-zero exit status: {}",
status.code().unwrap_or(1)
)))
} else {
Ok(())
}
self.execute_test_script(&source_dir, env, &rt)
}
}

#[async_trait::async_trait]
impl<'a> Tester for PackageInstallTester<'a> {
async fn test(&mut self) -> Result<()> {
self.test().await
}
fn prefix(&self) -> &Path {
&self.prefix
}
fn script(&self) -> &String {
&self.script
}
}
2 changes: 2 additions & 0 deletions crates/spk-cli/cmd-test/src/test/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
mod build;
mod install;
mod sources;
mod tester;

pub use build::PackageBuildTester;
pub use install::PackageInstallTester;
pub use sources::PackageSourceTester;
pub use tester::Tester;
Loading

0 comments on commit 60ccec5

Please sign in to comment.