From d09804846d77cbcbc74e01e4a4b63203f11bf1bf Mon Sep 17 00:00:00 2001 From: Sean Leather Date: Mon, 8 Jul 2019 10:26:18 +0200 Subject: [PATCH 1/3] Move env var tests into separate modules Tests that modify environment variables (e.g. CFLAGS and CXXFLAGS) can cause tests to fail when run in parallel because they change a shared context. This change moves the problematic tests into separate modules. Since each `tests/*.rs` is compiled as a separate crate, these tests are not run in parallel with other tests. --- tests/cflags.rs | 18 ++++++++++++++++++ tests/cxxflags.rs | 18 ++++++++++++++++++ tests/test.rs | 21 --------------------- 3 files changed, 36 insertions(+), 21 deletions(-) create mode 100644 tests/cflags.rs create mode 100644 tests/cxxflags.rs diff --git a/tests/cflags.rs b/tests/cflags.rs new file mode 100644 index 0000000..df6b0a7 --- /dev/null +++ b/tests/cflags.rs @@ -0,0 +1,18 @@ +extern crate cc; +extern crate tempdir; + +mod support; + +use std::env; +use support::Test; + +/// This test is in its own module because it modifies the environment and would affect other tests +/// when run in parallel with them. +#[test] +fn gnu_no_warnings_if_cflags() { + env::set_var("CFLAGS", "-arbitrary"); + let test = Test::gnu(); + test.gcc().file("foo.c").compile("foo"); + + test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra"); +} diff --git a/tests/cxxflags.rs b/tests/cxxflags.rs new file mode 100644 index 0000000..26426af --- /dev/null +++ b/tests/cxxflags.rs @@ -0,0 +1,18 @@ +extern crate cc; +extern crate tempdir; + +mod support; + +use std::env; +use support::Test; + +/// This test is in its own module because it modifies the environment and would affect other tests +/// when run in parallel with them. +#[test] +fn gnu_no_warnings_if_cxxflags() { + env::set_var("CXXFLAGS", "-arbitrary"); + let test = Test::gnu(); + test.gcc().file("foo.cpp").cpp(true).compile("foo"); + + test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra"); +} diff --git a/tests/test.rs b/tests/test.rs index 5147b77..8664165 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,7 +1,6 @@ extern crate cc; extern crate tempdir; -use std::env; use support::Test; mod support; @@ -111,26 +110,6 @@ fn gnu_warnings_overridable() { .must_have_in_order("-Wall", "-Wno-missing-field-initializers"); } -#[test] -fn gnu_no_warnings_if_cflags() { - env::set_var("CFLAGS", "-Wflag-does-not-exist"); - let test = Test::gnu(); - test.gcc().file("foo.c").compile("foo"); - - test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra"); - env::set_var("CFLAGS", ""); -} - -#[test] -fn gnu_no_warnings_if_cxxflags() { - env::set_var("CXXFLAGS", "-Wflag-does-not-exist"); - let test = Test::gnu(); - test.gcc().file("foo.c").compile("foo"); - - test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra"); - env::set_var("CXXFLAGS", ""); -} - #[test] fn gnu_x86_64() { for vendor in &["unknown-linux-gnu", "apple-darwin"] { From 8156af3a28497207a863d95da90659076142ea7b Mon Sep 17 00:00:00 2001 From: Sean Leather Date: Fri, 12 Jul 2019 09:55:07 +0200 Subject: [PATCH 2/3] Don't hard link the gcc-shim executable on macOS Fixes #419 --- tests/support/mod.rs | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 8a24ccd..7d74719 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -3,8 +3,9 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; +use std::io; use std::io::prelude::*; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use cc; use tempdir::TempDir; @@ -49,10 +50,13 @@ impl Test { } pub fn shim(&self, name: &str) -> &Test { - let fname = format!("{}{}", name, env::consts::EXE_SUFFIX); - fs::hard_link(&self.gcc, self.td.path().join(&fname)) - .or_else(|_| fs::copy(&self.gcc, self.td.path().join(&fname)).map(|_| ())) - .unwrap(); + link_or_copy( + &self.gcc, + self.td + .path() + .join(&format!("{}{}", name, env::consts::EXE_SUFFIX)), + ) + .unwrap(); self } @@ -136,3 +140,22 @@ impl Execution { self } } + +/// Hard link an executable or copy it if that fails. +/// +/// We first try to hard link an executable to save space. If that fails (as on Windows with +/// different mount points, issue #60), we copy. +#[cfg(not(target_os = "macos"))] +fn link_or_copy, Q: AsRef>(from: P, to: Q) -> io::Result<()> { + let from = from.as_ref(); + let to = to.as_ref(); + fs::hard_link(from, to).or_else(|_| fs::copy(from, to).map(|_| ())) +} + +/// Copy an executable. +/// +/// On macOS, hard linking the executable leads to strange failures (issue #419), so we just copy. +#[cfg(target_os = "macos")] +fn link_or_copy, Q: AsRef>(from: P, to: Q) -> io::Result<()> { + fs::copy(from, to).map(|_| ()) +} From 5acdb66980403878c7b5b2f66010c6aae2f3ed20 Mon Sep 17 00:00:00 2001 From: Sean Leather Date: Fri, 12 Jul 2019 09:56:51 +0200 Subject: [PATCH 3/3] Remove `--test-threads 1` on Azure --- ci/azure-steps.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/azure-steps.yml b/ci/azure-steps.yml index 9f418fb..bbf8ec6 100644 --- a/ci/azure-steps.yml +++ b/ci/azure-steps.yml @@ -16,9 +16,9 @@ steps: - script: cargo build displayName: "Normal build" - - bash: cargo test $NO_RUN -- --test-threads 1 + - bash: cargo test $NO_RUN displayName: "Crate tests" - - bash: cargo test $NO_RUN --features parallel -- --test-threads 1 + - bash: cargo test $NO_RUN --features parallel displayName: "Crate tests (parallel)" - bash: cargo test $NO_RUN --manifest-path cc-test/Cargo.toml --target $TARGET displayName: "cc-test tests"