Skip to content

Commit

Permalink
Fix race between tests that try to bind to a port.
Browse files Browse the repository at this point in the history
The original code would create a TcpListener and throw it away, which meant
that multiple calls to `get_unused_port` could end up with the same port.

Also, enable all the edgelet-docker tests that had been disabled on Windows
due to the original issue (even though it was not a Windows-specific problem).

However this means that the number of tests in the edgelet-docker crate running
at the same time is much higher, so it's more likely for the tests to hit
the issue described in rust-lang/backtrace-rs#230 (comment)
Specifically, any test that panics at the same time as another test is
using `failure` to collect a backtrace has a chance of crashing the entire
test binary with STATUS_HEAP_CORRUPTION on Windows, if the `RUST_BACKTRACE`
env var is set.

To avoid this, I've modified the tests that use
`#[should_panic]` + `Result::unwrap` to assert that an operation must fail to
instead use `Result::unwrap_err`.
  • Loading branch information
arsing committed Aug 13, 2019
1 parent 0ae9a09 commit 349e121
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 225 deletions.
7 changes: 3 additions & 4 deletions edgelet/edgelet-docker/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,14 @@ mod tests {
use super::*;

#[test]
#[should_panic]
fn empty_image_fails() {
DockerConfig::new("".to_string(), ContainerCreateBody::new(), None).unwrap();
let _ = DockerConfig::new("".to_string(), ContainerCreateBody::new(), None).unwrap_err();
}

#[test]
#[should_panic]
fn white_space_image_fails() {
DockerConfig::new(" ".to_string(), ContainerCreateBody::new(), None).unwrap();
let _ =
DockerConfig::new(" ".to_string(), ContainerCreateBody::new(), None).unwrap_err();
}

#[test]
Expand Down
10 changes: 4 additions & 6 deletions edgelet/edgelet-docker/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,25 +242,23 @@ mod tests {
}

#[test]
#[should_panic]
fn empty_name_fails() {
let _docker_module = DockerModule::new(
let _ = DockerModule::new(
create_api_client("boo"),
"".to_string(),
DockerConfig::new("ubuntu".to_string(), ContainerCreateBody::new(), None).unwrap(),
)
.unwrap();
.unwrap_err();
}

#[test]
#[should_panic]
fn white_space_name_fails() {
let _docker_module = DockerModule::new(
let _ = DockerModule::new(
create_api_client("boo"),
" ".to_string(),
DockerConfig::new("ubuntu".to_string(), ContainerCreateBody::new(), None).unwrap(),
)
.unwrap();
.unwrap_err();
}

fn get_inputs() -> Vec<(&'static str, i64, ModuleStatus)> {
Expand Down
14 changes: 8 additions & 6 deletions edgelet/edgelet-docker/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,30 +972,32 @@ mod tests {
}

#[test]
#[should_panic(expected = "URL does not have a recognized scheme")]
fn invalid_uri_prefix_fails() {
let settings = make_settings(Some(json!({
"moby_runtime": {
"uri": "foo:///this/is/not/valid"
}
})));
let _runtime = DockerModuleRuntime::make_runtime(settings, provisioning_result(), crypto())
let err = DockerModuleRuntime::make_runtime(settings, provisioning_result(), crypto())
.wait()
.unwrap();
.unwrap_err();
assert!(err
.to_string()
.contains("URL does not have a recognized scheme"));
}

#[cfg(unix)]
#[test]
#[should_panic(expected = "Socket file could not be found")]
fn invalid_uds_path_fails() {
let settings = make_settings(Some(json!({
"moby_runtime": {
"uri": "unix:///this/file/does/not/exist"
}
})));
let _runtime = DockerModuleRuntime::make_runtime(settings, provisioning_result(), crypto())
let err = DockerModuleRuntime::make_runtime(settings, provisioning_result(), crypto())
.wait()
.unwrap();
.unwrap_err();
assert!(err.to_string().contains("Socket file could not be found"));
}

#[test]
Expand Down
Loading

0 comments on commit 349e121

Please sign in to comment.