Skip to content

Commit

Permalink
Fix race between tests that try to bind to a port. (Azure#1566)
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 assert that an operation must fail
via `#[should_panic]` + `Result::unwrap` to instead use `Result::unwrap_err`
  • Loading branch information
arsing authored Aug 14, 2019
1 parent cc351e4 commit 9036821
Show file tree
Hide file tree
Showing 19 changed files with 330 additions and 350 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
19 changes: 13 additions & 6 deletions edgelet/edgelet-docker/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ pub struct DockerModule<C: Connect> {
config: DockerConfig,
}

impl<C> std::fmt::Debug for DockerModule<C>
where
C: Connect,
{
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("DockerModule").finish()
}
}

impl<C: 'static + Connect> DockerModule<C> {
pub fn new(client: DockerClient<C>, name: String, config: DockerConfig) -> Result<Self> {
ensure_not_empty_with_context(&name, || ErrorKind::InvalidModuleName(name.clone()))?;
Expand Down Expand Up @@ -242,25 +251,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
21 changes: 15 additions & 6 deletions edgelet/edgelet-docker/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ impl DockerModuleRuntime {
}
}

impl std::fmt::Debug for DockerModuleRuntime {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("DockerModuleRuntime").finish()
}
}

impl ModuleRegistry for DockerModuleRuntime {
type Error = Error;
type PullFuture = Box<dyn Future<Item = (), Error = Self::Error> + Send>;
Expand Down Expand Up @@ -972,30 +978,33 @@ 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!(failure::Fail::iter_chain(&err).any(|err| 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!(failure::Fail::iter_chain(&err)
.any(|err| err.to_string().contains("Socket file could not be found")));
}

#[test]
Expand Down
Loading

0 comments on commit 9036821

Please sign in to comment.