Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history 
* updated error enum names
* removed need for wrapper struct for packaged buildpacks
  • Loading branch information
@colincasey
colincasey committed Sep 18, 2023
1 parent 3863a69 commit fa60158
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 52 deletions.
58 changes: 19 additions & 39 deletions libcnb-test/src/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,15 @@ use libcnb_package::package::PackageBuildpackError;
use libcnb_package::{find_cargo_workspace_root_dir, CargoProfile, FindCargoWorkspaceRootError};
use std::collections::BTreeMap;
use std::fs;
use std::path::PathBuf;
use tempfile::{tempdir, TempDir};
use std::path::{Path, PathBuf};

/// Packages the current crate as a buildpack into a temporary directory.
pub(crate) fn package_crate_buildpack(
cargo_profile: CargoProfile,
target_triple: impl AsRef<str>,
) -> Result<PackagedBuildpackDir, PackageTestBuildpackError> {
let cargo_manifest_dir = std::env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.map_err(PackageTestBuildpackError::CannotDetermineCrateDirectory)?;

cargo_manifest_dir: &Path,
target_buildpack_dir: &Path,
) -> Result<PathBuf, PackageTestBuildpackError> {
let buildpack_toml = cargo_manifest_dir.join("buildpack.toml");

assert!(
Expand All @@ -30,25 +27,25 @@ pub(crate) fn package_crate_buildpack(
cargo_manifest_dir.display()
);

let buildpack_descriptor: BuildpackDescriptor =
read_toml_file(buildpack_toml).map_err(PackageTestBuildpackError::CannotReadBuildpack)?;
let buildpack_descriptor: BuildpackDescriptor = read_toml_file(buildpack_toml)
.map_err(PackageTestBuildpackError::CannotReadBuildpackDescriptor)?;

package_buildpack(
&buildpack_descriptor.buildpack().id,
cargo_profile,
target_triple,
cargo_manifest_dir,
target_buildpack_dir,
)
}

pub(crate) fn package_buildpack(
buildpack_id: &BuildpackId,
cargo_profile: CargoProfile,
target_triple: impl AsRef<str>,
) -> Result<PackagedBuildpackDir, PackageTestBuildpackError> {
let cargo_manifest_dir = std::env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.map_err(PackageTestBuildpackError::CannotDetermineCrateDirectory)?;

cargo_manifest_dir: &Path,
target_buildpack_dir: &Path,
) -> Result<PathBuf, PackageTestBuildpackError> {
let cargo_build_env = match cross_compile_assistance(target_triple.as_ref()) {
CrossCompileAssistance::HelpText(help_text) => {
return Err(PackageTestBuildpackError::CrossCompileConfigurationError(
Expand All @@ -59,25 +56,18 @@ pub(crate) fn package_buildpack(
CrossCompileAssistance::Configuration { cargo_env } => cargo_env,
};

let workspace_root_path = find_cargo_workspace_root_dir(&cargo_manifest_dir)
.map_err(PackageTestBuildpackError::FindCargoWorkspace)?;

let package_dir =
tempdir().map_err(PackageTestBuildpackError::CannotCreateBuildpackTempDirectory)?;
let workspace_root_path = find_cargo_workspace_root_dir(cargo_manifest_dir)
.map_err(PackageTestBuildpackError::FindCargoWorkspaceRoot)?;

let buildpack_dir_resolver = create_packaged_buildpack_dir_resolver(
package_dir.as_ref(),
target_buildpack_dir,
cargo_profile,
target_triple.as_ref(),
);

// TODO: this could accidentally detect a packaged meta-buildpack twice. how should we ignore directories
// containing packaged buildpacks that might get detected now that the user controls --package-dir?
// - support .gitignore or some other persistent configuration?
// - during packaging should we create some type of file that indicates the output is a packaged buildpack?
let buildpack_dependency_graph =
build_libcnb_buildpacks_dependency_graph(&workspace_root_path, &[])
.map_err(PackageTestBuildpackError::CreateBuildpackDependencyGraph)?;
.map_err(PackageTestBuildpackError::BuildBuildpackDependencyGraph)?;

let root_node = buildpack_dependency_graph
.node_weights()
Expand Down Expand Up @@ -114,25 +104,15 @@ pub(crate) fn package_buildpack(
packaged_buildpack_dirs.insert(node.buildpack_id.clone(), buildpack_destination_dir);
}

Ok(PackagedBuildpackDir {
_root: package_dir,
path: buildpack_dir_resolver(buildpack_id),
})
}

pub(crate) struct PackagedBuildpackDir {
_root: TempDir,
pub(crate) path: PathBuf,
Ok(buildpack_dir_resolver(buildpack_id))
}

#[derive(Debug)]
pub(crate) enum PackageTestBuildpackError {
CannotCreateBuildpackTempDirectory(std::io::Error),
CannotDetermineCrateDirectory(std::env::VarError),
CannotReadBuildpack(TomlFileError),
CreateBuildpackDependencyGraph(BuildBuildpackDependencyGraphError),
CannotReadBuildpackDescriptor(TomlFileError),
BuildBuildpackDependencyGraph(BuildBuildpackDependencyGraphError),
CrossCompileConfigurationError(String),
FindCargoWorkspace(FindCargoWorkspaceRootError),
FindCargoWorkspaceRoot(FindCargoWorkspaceRootError),
GetDependencies(GetDependenciesError<BuildpackId>),
PackageBuildpack(PackageBuildpackError),
}
36 changes: 23 additions & 13 deletions libcnb-test/src/test_runner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{app, build, util, BuildConfig, BuildpackReference, PackResult, TestC
use std::borrow::Borrow;
use std::env;
use std::path::PathBuf;
use tempfile::tempdir;

/// Runner for libcnb integration tests.
///
Expand Down Expand Up @@ -58,12 +59,13 @@ impl TestRunner {
) {
let config = config.borrow();

let cargo_manifest_dir = env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.expect("Could not determine Cargo manifest directory");

let app_dir = {
let normalized_app_dir_path = if config.app_dir.is_relative() {
env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.expect("Could not determine Cargo manifest directory")
.join(&config.app_dir)
cargo_manifest_dir.join(&config.app_dir)
} else {
config.app_dir.clone()
};
Expand All @@ -88,7 +90,8 @@ impl TestRunner {
}
};

let mut buildpack_dirs = vec![];
let buildpacks_target_dir =
tempdir().expect("Could not create a temporary directory for compiled buildpacks");

let mut pack_command = PackBuildCommand::new(&config.builder_name, &app_dir, &image_name);

Expand All @@ -99,17 +102,24 @@ impl TestRunner {
for buildpack in &config.buildpacks {
match buildpack {
BuildpackReference::CurrentCrate => {
let crate_buildpack_dir = build::package_crate_buildpack(config.cargo_profile, &config.target_triple)
.expect("Test references crate buildpack, but crate wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences");
pack_command.buildpack(crate_buildpack_dir.path.clone());
buildpack_dirs.push(crate_buildpack_dir);
let crate_buildpack_dir = build::package_crate_buildpack(
config.cargo_profile,
&config.target_triple,
&cargo_manifest_dir,
buildpacks_target_dir.path(),
).expect("Test references crate buildpack, but crate wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences");
pack_command.buildpack(crate_buildpack_dir);
}

BuildpackReference::WorkspaceBuildpack(builpack_id) => {
let buildpack_dir = build::package_buildpack(builpack_id, config.cargo_profile, &config.target_triple)
.unwrap_or_else(|_| panic!("Test references buildpack `{builpack_id}`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"));
pack_command.buildpack(buildpack_dir.path.clone());
buildpack_dirs.push(buildpack_dir);
let buildpack_dir = build::package_buildpack(
builpack_id,
config.cargo_profile,
&config.target_triple,
&cargo_manifest_dir,
buildpacks_target_dir.path()
).unwrap_or_else(|_| panic!("Test references buildpack `{builpack_id}`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"));
pack_command.buildpack(buildpack_dir);
}

BuildpackReference::Other(id) => {
Expand Down

0 comments on commit fa60158

Please sign in to comment.