Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libcnb-test: Improve error messages for address_for_port #636

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<SocketAddr>`. ([#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<SocketAddr>`. 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))
Expand Down
2 changes: 1 addition & 1 deletion libcnb-test/src/container_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use std::collections::{HashMap, HashSet};
/// },
/// );
/// ```
#[derive(Default)]
#[derive(Clone, Default)]
pub struct ContainerConfig {
pub(crate) entrypoint: Option<String>,
pub(crate) command: Option<Vec<String>>,
Expand Down
35 changes: 26 additions & 9 deletions libcnb-test/src/container_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion libcnb-test/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 14 additions & 14 deletions libcnb-test/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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);
},
);
},
Expand Down