Fix error messages for buildpack packaging failures

CHANGELOG.md

@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- `libcnb-test`:
+  - Fixed incorrect error messages being shown for buildpack compilation/packaging failures. ([#720](https://github.com/heroku/libcnb.rs/pull/720))
 
 ## [0.15.0] - 2023-09-25
 

libcnb-test/Cargo.toml

@@ -18,6 +18,7 @@ libcnb-common.workspace = true
 libcnb-data.workspace = true
 libcnb-package.workspace = true
 tempfile = "3.8.0"
+thiserror = "1.0.48"
 
 [dev-dependencies]
 indoc = "2.0.4"

libcnb-test/src/build.rs

@@ -8,8 +8,8 @@ use libcnb_package::dependency_graph::{get_dependencies, GetDependenciesError};
 use libcnb_package::output::create_packaged_buildpack_dir_resolver;
 use libcnb_package::{find_cargo_workspace_root_dir, CargoProfile, FindCargoWorkspaceRootError};
 use std::collections::BTreeMap;
-use std::fs;
 use std::path::{Path, PathBuf};
+use std::{fs, io};
 
 /// Packages the current crate as a buildpack into the provided directory.
 // TODO: Convert the `assert!` usages to an additional `PackageBuildpackError` variant instead:
@@ -23,11 +23,11 @@ pub(crate) fn package_crate_buildpack(
 ) -> Result<PathBuf, PackageBuildpackError> {
     let buildpack_toml = cargo_manifest_dir.join("buildpack.toml");
 
-    assert!(
-        buildpack_toml.exists(),
-        "Could not package directory as buildpack! No `buildpack.toml` file exists at {}",
-        cargo_manifest_dir.display()
-    );
+    if !buildpack_toml.exists() {
+        return Err(PackageBuildpackError::BuildpackDescriptorNotFound(
+            buildpack_toml,
+        ));
+    }
 
     let buildpack_descriptor: BuildpackDescriptor = read_toml_file(buildpack_toml)
         .map_err(PackageBuildpackError::CannotReadBuildpackDescriptor)?;
@@ -53,7 +53,7 @@ pub(crate) fn package_buildpack(
 ) -> Result<PathBuf, PackageBuildpackError> {
     let cargo_build_env = match cross_compile_assistance(target_triple.as_ref()) {
         CrossCompileAssistance::HelpText(help_text) => {
-            return Err(PackageBuildpackError::CrossCompileConfigurationError(
+            return Err(PackageBuildpackError::CrossCompileToolchainNotFound(
                 help_text,
             ));
         }
@@ -75,28 +75,21 @@ pub(crate) fn package_buildpack(
 
     let root_node = buildpack_dependency_graph
         .node_weights()
-        .find(|node| node.buildpack_id == buildpack_id.clone());
+        .find(|node| &node.buildpack_id == buildpack_id)
+        .ok_or_else(|| {
+            PackageBuildpackError::BuildpackIdNotFound(buildpack_id.clone(), workspace_root_path)
+        })?;
 
-    assert!(
-        root_node.is_some(),
-        "Could not package directory as buildpack! No buildpack with id `{buildpack_id}` exists in the workspace at {}",
-        workspace_root_path.display()
-    );
-
-    let build_order = get_dependencies(
-        &buildpack_dependency_graph,
-        &[root_node.expect("The root node should exist")],
-    )
-    .map_err(PackageBuildpackError::GetDependencies)?;
+    let build_order = get_dependencies(&buildpack_dependency_graph, &[root_node])
+        .map_err(PackageBuildpackError::GetDependencies)?;
 
     let mut packaged_buildpack_dirs = BTreeMap::new();
     for node in &build_order {
         let buildpack_destination_dir = buildpack_dir_resolver(&node.buildpack_id);
 
-        // TODO: Convert the `unwrap()` to an additional `PackageBuildpackError` variant instead:
-        // https://github.com/heroku/libcnb.rs/issues/710
-        #[allow(clippy::unwrap_used)]
-        fs::create_dir_all(&buildpack_destination_dir).unwrap();
+        fs::create_dir_all(&buildpack_destination_dir).map_err(|error| {
+            PackageBuildpackError::CannotCreateDirectory(buildpack_destination_dir.clone(), error)
+        })?;
 
         libcnb_package::package::package_buildpack(
             &node.path,
@@ -114,12 +107,24 @@ pub(crate) fn package_buildpack(
     Ok(buildpack_dir_resolver(buildpack_id))
 }
 
-#[derive(Debug)]
+#[derive(thiserror::Error, Debug)]
 pub(crate) enum PackageBuildpackError {
+    #[error("Couldn't find a buildpack.toml file at {0}")]
+    BuildpackDescriptorNotFound(PathBuf),
+    #[error("Couldn't find a buildpack with ID '{0}' in the workspace at {1}")]
+    BuildpackIdNotFound(BuildpackId, PathBuf),
+    #[error("I/O error creating directory {0}: {1}")]
+    CannotCreateDirectory(PathBuf, io::Error),
+    #[error("Couldn't read buildpack.toml: {0}")]
     CannotReadBuildpackDescriptor(TomlFileError),
+    #[error("Couldn't calculate buildpack dependency graph: {0}")]
     BuildBuildpackDependencyGraph(BuildBuildpackDependencyGraphError),
-    CrossCompileConfigurationError(String),
+    #[error("Couldn't find cross-compilation toolchain.\n\n{0}")]
+    CrossCompileToolchainNotFound(String),
+    #[error("Couldn't find Cargo workspace root: {0}")]
     FindCargoWorkspaceRoot(FindCargoWorkspaceRootError),
+    #[error("Couldn't get buildpack dependencies: {0}")]
     GetDependencies(GetDependenciesError<BuildpackId>),
+    #[error("{0}")]
     PackageBuildpack(libcnb_package::package::PackageBuildpackError),
 }

libcnb-test/src/test_runner.rs

@@ -107,7 +107,10 @@ impl TestRunner {
                         &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");
+                    )
+                    .unwrap_or_else(|error| {
+                        panic!("Error packaging current crate as buildpack: {error}")
+                    });
                     pack_command.buildpack(crate_buildpack_dir);
                 }
 
@@ -117,8 +120,11 @@ impl TestRunner {
                         config.cargo_profile,
                         &config.target_triple,
                         &cargo_manifest_dir,
-                        buildpacks_target_dir.path()
-                    ).unwrap_or_else(|_| panic!("Test references buildpack `{buildpack_id}`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"));
+                        buildpacks_target_dir.path(),
+                    )
+                    .unwrap_or_else(|error| {
+                        panic!("Error packaging buildpack '{buildpack_id}': {error}")
+                    });
                     pack_command.buildpack(buildpack_dir);
                 }
 

libcnb-test/tests/integration_test.rs

@@ -146,8 +146,12 @@ fn packaging_failure_missing_buildpack_toml() {
     assert_eq!(
         err.downcast_ref::<String>().unwrap(),
         &format!(
-            "Could not package directory as buildpack! No `buildpack.toml` file exists at {}",
-            env::var("CARGO_MANIFEST_DIR").unwrap()
+            "Error packaging current crate as buildpack: Couldn't find a buildpack.toml file at {}",
+            env::var("CARGO_MANIFEST_DIR")
+                .map(PathBuf::from)
+                .unwrap()
+                .join("buildpack.toml")
+                .display()
         )
     );
 }
@@ -157,7 +161,7 @@ fn packaging_failure_missing_buildpack_toml() {
 // TODO: Fix the implementation to return the correct error message (that the buildpack.toml doesn't match the schema):
 // https://github.com/heroku/libcnb.rs/issues/708
 #[should_panic(
-    expected = "Could not package directory as buildpack! No buildpack with id `libcnb-test/invalid-buildpack-toml` exists in the workspace at"
+    expected = "Error packaging buildpack 'libcnb-test/invalid-buildpack-toml': Couldn't find a buildpack with ID 'libcnb-test/invalid-buildpack-toml' in the workspace at"
 )]
 fn packaging_failure_invalid_buildpack_toml() {
     TestRunner::default().build(
@@ -174,10 +178,8 @@ fn packaging_failure_invalid_buildpack_toml() {
 
 #[test]
 #[ignore = "integration test"]
-// TODO: Fix the implementation to return the correct error message. Has the same cause as:
-// https://github.com/heroku/libcnb.rs/issues/704
 #[should_panic(
-    expected = "Test references buildpack `libcnb-test/composite-missing-package-toml`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"
+    expected = "Error packaging buildpack 'libcnb-test/composite-missing-package-toml': Could not read package.toml: IO error while reading/writing TOML file: No such file or directory (os error 2)"
 )]
 fn packaging_failure_composite_buildpack_missing_package_toml() {
     TestRunner::default().build(
@@ -194,10 +196,8 @@ fn packaging_failure_composite_buildpack_missing_package_toml() {
 
 #[test]
 #[ignore = "integration test"]
-// TODO: Fix the implementation to return the correct error message. Has the same cause as:
-// https://github.com/heroku/libcnb.rs/issues/704
 #[should_panic(
-    expected = "Test references buildpack `libcnb-test/invalid-cargo-toml`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"
+    expected = "Error packaging buildpack 'libcnb-test/invalid-cargo-toml': Obtaining Cargo metadata failed: `cargo metadata` exited with an error: "
 )]
 fn packaging_failure_invalid_cargo_toml() {
     TestRunner::default().build(
@@ -212,10 +212,8 @@ fn packaging_failure_invalid_cargo_toml() {
 
 #[test]
 #[ignore = "integration test"]
-// TODO: Fix the implementation to return the correct error message. Has the same cause as:
-// https://github.com/heroku/libcnb.rs/issues/704
 #[should_panic(
-    expected = "Test references buildpack `libcnb-test/compile-error`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"
+    expected = "Error packaging buildpack 'libcnb-test/compile-error': Building buildpack binaries failed: Failed to build binary target compile-error: Cargo unexpectedly exited with status exit status: 101"
 )]
 fn packaging_failure_compile_error() {
     TestRunner::default().build(
@@ -246,7 +244,7 @@ fn packaging_failure_non_existent_workspace_buildpack() {
     assert_eq!(
         err.downcast_ref::<String>().unwrap(),
         &format!(
-            "Could not package directory as buildpack! No buildpack with id `non-existent` exists in the workspace at {}",
+            "Error packaging buildpack 'non-existent': Couldn't find a buildpack with ID 'non-existent' in the workspace at {}",
             // There is currently no env var for determining the workspace root directly:
             // https://github.com/rust-lang/cargo/issues/3946
             env::var("CARGO_MANIFEST_DIR")
@@ -320,7 +318,7 @@ fn expected_pack_failure() {
 #[test]
 #[ignore = "integration test"]
 #[should_panic(
-    expected = "Could not package directory as buildpack! No `buildpack.toml` file exists at"
+    expected = "Error packaging current crate as buildpack: Couldn't find a buildpack.toml file"
 )]
 fn expected_pack_failure_still_panics_for_non_pack_failure() {
     TestRunner::default().build(