diff --git a/CHANGELOG.md b/CHANGELOG.md index 413ba5ff..b3392f50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,12 +9,14 @@ separate changelogs for each crate were used. If you need to refer to these old ### Added - `libcnb-package`: Add cross-compilation assistance for Linux `aarch64-unknown-linux-musl`. ([#577](https://github.com/heroku/libcnb.rs/pull/577)) -- `libcnb-test`: `LogOutput` now implements `std::fmt::Display`. ([#635](https://github.com/heroku/libcnb.rs/pull/635)) +- `libcnb-test`: + - `LogOutput` now implements `std::fmt::Display`. ([#635](https://github.com/heroku/libcnb.rs/pull/635)) + - `ContainerConfig` now implements `Clone`. ([#636](https://github.com/heroku/libcnb.rs/pull/636)) ### Changed - `libcnb-test`: - - `ContainerContext::address_for_port` now returns `SocketAddr` directly instead of `Option`. ([#605](https://github.com/heroku/libcnb.rs/pull/605)) + - `ContainerContext::address_for_port` will now panic for all failure modes rather than just some, and so now returns `SocketAddr` directly instead of `Option`. This reduces test boilerplate due to the caller no longer needing to `.unwrap()` and improves debugging UX when containers crash after startup. ([#605](https://github.com/heroku/libcnb.rs/pull/605) and [#636](https://github.com/heroku/libcnb.rs/pull/636)) - Docker commands are now run using the Docker CLI instead of Bollard and the Docker daemon API. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) - `ContainerConfig::entrypoint` now accepts a String rather than a vector of strings. Any arguments to the entrypoint should be moved to `ContainerConfig::command`. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) - `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)) diff --git a/libcnb-test/src/container_config.rs b/libcnb-test/src/container_config.rs index c3828db4..f98f8b48 100644 --- a/libcnb-test/src/container_config.rs +++ b/libcnb-test/src/container_config.rs @@ -25,7 +25,7 @@ use std::collections::{HashMap, HashSet}; /// }, /// ); /// ``` -#[derive(Default)] +#[derive(Clone, Default)] pub struct ContainerConfig { pub(crate) entrypoint: Option, pub(crate) command: Option>, diff --git a/libcnb-test/src/container_context.rs b/libcnb-test/src/container_context.rs index 882968a1..5fa2146f 100644 --- a/libcnb-test/src/container_context.rs +++ b/libcnb-test/src/container_context.rs @@ -2,13 +2,15 @@ use crate::docker::{ DockerExecCommand, DockerLogsCommand, DockerPortCommand, DockerRemoveContainerCommand, }; use crate::log::LogOutput; -use crate::util; +use crate::util::CommandError; +use crate::{util, ContainerConfig}; use std::net::SocketAddr; /// Context of a launched container. pub struct ContainerContext { /// The randomly generated name of this container. pub container_name: String, + pub(crate) config: ContainerConfig, } impl ContainerContext { @@ -111,15 +113,30 @@ impl ContainerContext { /// was not exposed using [`ContainerConfig::expose_port`](crate::ContainerConfig::expose_port). #[must_use] pub fn address_for_port(&self, port: u16) -> SocketAddr { + assert!( + self.config.exposed_ports.contains(&port), + "Unknown port: Port {port} needs to be exposed first using `ContainerConfig::expose_port`" + ); + let docker_port_command = DockerPortCommand::new(&self.container_name, port); - util::run_command(docker_port_command) - .unwrap_or_else(|command_err| { - panic!("Error obtaining container port mapping:\n\n{command_err}") - }) - .stdout - .trim() - .parse() - .expect("Couldn't parse `docker port` output") + + match util::run_command(docker_port_command) { + Ok(output) => output + .stdout + .trim() + .parse() + .expect("Couldn't parse `docker port` output"), + Err(CommandError::NonZeroExitCode { log_output, .. }) => { + panic!( + "Error obtaining container port mapping:\n{}\nThis normally means that the container crashed. Container logs:\n\n{}", + log_output.stderr, + self.logs_now() + ); + } + Err(command_err) => { + panic!("Error obtaining container port mapping:\n\n{command_err}"); + } + } } /// Executes a shell command inside an already running container. diff --git a/libcnb-test/src/test_context.rs b/libcnb-test/src/test_context.rs index e838516d..bc08deb6 100644 --- a/libcnb-test/src/test_context.rs +++ b/libcnb-test/src/test_context.rs @@ -109,7 +109,10 @@ impl<'a> TestContext<'a> { util::run_command(docker_run_command) .unwrap_or_else(|command_err| panic!("Error starting container:\n\n{command_err}")); - f(ContainerContext { container_name }); + f(ContainerContext { + container_name, + config: config.clone(), + }); } /// Run the provided shell command. diff --git a/libcnb-test/tests/integration_test.rs b/libcnb-test/tests/integration_test.rs index 36e8bc85..76bfaedc 100644 --- a/libcnb-test/tests/integration_test.rs +++ b/libcnb-test/tests/integration_test.rs @@ -475,13 +475,9 @@ fn expose_port_invalid_port() { #[test] #[ignore = "integration test"] -#[should_panic(expected = "Error obtaining container port mapping: - -docker command failed with exit code 1! - -## stderr: - -Error: No public port '12345")] +#[should_panic( + expected = "Unknown port: Port 12345 needs to be exposed first using `ContainerConfig::expose_port`" +)] fn address_for_port_when_port_not_exposed() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -496,14 +492,17 @@ fn address_for_port_when_port_not_exposed() { #[test] #[ignore = "integration test"] -// TODO: Improve the UX here: https://github.com/heroku/libcnb.rs/issues/482 -#[should_panic(expected = "Error obtaining container port mapping: - -docker command failed with exit code 1! +#[should_panic(expected = " +This normally means that the container crashed. Container logs: ## stderr: -Error: No public port '12345")] +some stderr + +## stdout: + +some stdout +")] fn address_for_port_when_container_crashed() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -512,11 +511,12 @@ fn address_for_port_when_container_crashed() { context.start_container( ContainerConfig::new() .entrypoint("launcher") - .command(["exit 1"]) + .command(["echo 'some stdout'; echo 'some stderr' >&2; exit 1"]) .expose_port(TEST_PORT), |container| { + // Wait for the container to actually exit, otherwise `address_for_port()` will succeed. thread::sleep(Duration::from_secs(1)); - let _ = container.address_for_port(12345); + let _ = container.address_for_port(TEST_PORT); }, ); },