Skip to content

Commit

Permalink
libcnb-test: Fix image cleanup when a pack build unexpectedly succeeds (
Browse files Browse the repository at this point in the history
#625)

Previously, if a test with an expected result of `PackResult::Failure`
unexpectedly succeeded, the built app image was left behind since
cleanup was only performed in `TestContext::Drop` (and in this
scenario, a `TestContext` is never created in the first place).

Now the app image is correctly cleaned up.

Fixes #586.
Closes #588.
GUS-W-13903181.
  • Loading branch information
edmorley authored Aug 8, 2023
1 parent 2fe526c commit 3332439
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ separate changelogs for each crate were used. If you need to refer to these old
- `TestRunner::new` has been removed, since its only purpose was for advanced configuration that's no longer applicable. Use `TestRunner::default` instead. ([#620](https://github.com/heroku/libcnb.rs/pull/620))
- `LogOutput` no longer exposes `stdout_raw` and `stderr_raw`. ([#607](https://github.com/heroku/libcnb.rs/pull/607))
- Improved wording of panic error messages. ([#619](https://github.com/heroku/libcnb.rs/pull/619) and [#620](https://github.com/heroku/libcnb.rs/pull/620))
- If a test with an expected result of `PackResult::Failure` unexpectedly succeeds, the built app image is now correctly cleaned up. ([#625](https://github.com/heroku/libcnb.rs/pull/625))
- `libcnb-package`: buildpack target directory now contains the target triple. Users that implicitly rely on the output directory need to adapt. The output of `cargo libcnb package` will refer to the new locations. ([#580](https://github.com/heroku/libcnb.rs/pull/580))
- `libherokubuildpack`: Switch the `flate2` decompression backend from `miniz_oxide` to `zlib`. ([#593](https://github.com/heroku/libcnb.rs/pull/593))
- Bump minimum external dependency versions. ([#587](https://github.com/heroku/libcnb.rs/pull/587))
Expand Down
15 changes: 11 additions & 4 deletions libcnb-test/src/test_runner.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::docker::DockerRemoveImageCommand;
use crate::pack::PackBuildCommand;
use crate::util::CommandError;
use crate::{
Expand Down Expand Up @@ -114,15 +115,21 @@ impl TestRunner {
};
}

let output = match (
&config.expected_pack_result,
util::run_command(pack_command),
) {
let pack_result = util::run_command(pack_command);

let output = match (&config.expected_pack_result, pack_result) {
(PackResult::Success, Ok(output)) => output,
(PackResult::Failure, Err(CommandError::NonZeroExitCode { stdout, stderr, .. })) => {
LogOutput { stdout, stderr }
}
(PackResult::Failure, Ok(LogOutput { stdout, stderr })) => {
// Ordinarily the Docker image created by `pack build` will either be cleaned up by
// `TestContext::Drop` later on, or will not have been created in the first place,
// if the `pack build` was not successful. However, in the case of an unexpectedly
// successful `pack build` we have to clean this image up manually before `panic`ing.
util::run_command(DockerRemoveImageCommand::new(image_name)).unwrap_or_else(
|command_err| panic!("Error removing Docker image:\n\n{command_err}"),
);
panic!("The pack build was expected to fail, but did not:\n\n## stderr:\n\n{stderr}\n## stdout:\n\n{stdout}\n");
}
(_, Err(command_err)) => {
Expand Down

0 comments on commit 3332439

Please sign in to comment.