Merge pull request rust-lang#93 from gnzlbg/fix_exit_code

Fix exit code of rust-semverver in tests

ci/run.sh

@@ -2,6 +2,9 @@
 
 set -ex
 
+# Note: this is required for correctness,
+# otherwise executing multiple "full" tests in parallel
+# of the same library can alter results.
 export RUST_TEST_THREADS=1
 export RUST_BACKTRACE=full
 #export RUST_TEST_NOCAPTURE=1

src/bin/cargo_semver.rs

@@ -11,9 +11,9 @@ use cargo::core::{Package, PackageId, PackageSet, Source, SourceId, SourceMap, W
 use log::debug;
 use std::{
     env,
+    fs::File,
     io::BufReader,
     io::Write,
-    fs::File,
     path::{Path, PathBuf},
     process::{Command, Stdio},
 };
@@ -23,6 +23,7 @@ pub type Result<T> = cargo::util::CargoResult<T>;
 #[derive(Debug, Deserialize)]
 struct Invocation {
     package_name: String,
+    target_kind: Vec<String>,
     outputs: Vec<PathBuf>,
 }
 
@@ -120,8 +121,10 @@ fn run(config: &cargo::Config, matches: &getopts::Matches, explain: bool) -> Res
         (work_info, stable_crate.max_version.clone())
     };
 
-    let (current_rlib, current_deps_output) = current.rlib_and_dep_output(config, &name, true)?;
-    let (stable_rlib, stable_deps_output) = stable.rlib_and_dep_output(config, &name, false)?;
+    let (current_rlib, current_deps_output) =
+        current.rlib_and_dep_output(config, &name, true, matches)?;
+    let (stable_rlib, stable_deps_output) =
+        stable.rlib_and_dep_output(config, &name, false, matches)?;
 
     println!("current_rlib: {:?}", current_rlib);
     println!("stable_rlib: {:?}", stable_rlib);
@@ -151,6 +154,10 @@ fn run(config: &cargo::Config, matches: &getopts::Matches, explain: bool) -> Res
         child.args(&["--target", &target]);
     }
 
+    if !matches.opt_present("no-default-features") {
+        child.args(&["--cfg", "feature=\"default\""]);
+    }
+
     let child = child
         .arg("-")
         .stdin(Stdio::piped())
@@ -179,14 +186,20 @@ fn run(config: &cargo::Config, matches: &getopts::Matches, explain: bool) -> Res
              extern crate new;"
         ))?;
     } else {
-        return Err(failure::err_msg("could not pipe to rustc (wtf?)".to_owned()).into());
+        return Err(failure::err_msg(
+            "could not pipe to rustc (wtf?)".to_owned(),
+        ));
     }
 
-    child
+    let exit_status = child
         .wait()
         .map_err(|e| failure::err_msg(format!("failed to wait for rustc: {}", e)))?;
 
-    Ok(())
+    if exit_status.success() {
+        Ok(())
+    } else {
+        Err(failure::err_msg("rustc-semverver errored".to_owned()))
+    }
 }
 
 /// CLI utils
@@ -207,6 +220,11 @@ mod cli {
             "api-guidelines",
             "report only changes that are breaking according to the API-guidelines",
         );
+        opts.optflag(
+            "",
+            "no-default-features",
+            "Do not activate the `default` feature",
+        );
         opts.optopt(
             "s",
             "stable-path",
@@ -379,15 +397,24 @@ impl<'a> WorkInfo<'a> {
         config: &'a cargo::Config,
         name: &str,
         current: bool,
+        matches: &getopts::Matches,
     ) -> Result<(PathBuf, PathBuf)> {
         let mut opts =
             cargo::ops::CompileOptions::new(config, cargo::core::compiler::CompileMode::Build)?;
         // we need the build plan to find our build artifacts
         opts.build_config.build_plan = true;
+
+        if let Some(target) = matches.opt_str("target") {
+            opts.build_config.requested_target = Some(target);
+        }
+        opts.no_default_features = matches.opt_present("no-default-features");
+
         // TODO: this is where we could insert feature flag builds (or using the CLI mechanisms)
 
-        env::set_var("RUSTFLAGS",
-                     format!("-C metadata={}", if current { "new" } else { "old" }));
+        env::set_var(
+            "RUSTFLAGS",
+            format!("-C metadata={}", if current { "new" } else { "old" }),
+        );
 
         let mut outdir = env::temp_dir();
         outdir.push(&format!("cargo_semver_{}_{}", name, current));
@@ -406,13 +433,14 @@ impl<'a> WorkInfo<'a> {
         let compilation = cargo::ops::compile(&self.workspace, &opts)?;
         env::remove_var("RUSTFLAGS");
 
-        let build_plan: BuildPlan =
-            serde_json::from_reader(BufReader::new(File::open(&outdir)?))?;
+        let build_plan: BuildPlan = serde_json::from_reader(BufReader::new(File::open(&outdir)?))?;
 
         // TODO: handle multiple outputs gracefully
         for i in &build_plan.invocations {
-            if i.package_name == name {
-                return Ok((i.outputs[0].clone(), compilation.deps_output));
+            if let Some(kind) = i.target_kind.get(0) {
+                if kind.contains("lib") && i.package_name == name {
+                    return Ok((i.outputs[0].clone(), compilation.deps_output));
+                }
             }
         }
 
@@ -433,15 +461,13 @@ pub fn find_on_crates_io(crate_name: &str) -> Result<crates_io::Crate> {
                 "failed to retrieve search results from the registry: {}",
                 e
             ))
-            .into()
         })
         .and_then(|(mut crates, _)| {
             crates
                 .drain(..)
                 .find(|krate| krate.name == crate_name)
                 .ok_or_else(|| {
                     failure::err_msg(format!("failed to find a matching crate `{}`", crate_name))
-                        .into()
                 })
         })
 }

src/bin/rust_semverver.rs

@@ -1,5 +1,4 @@
 #![feature(rustc_private)]
-#![feature(try_from)]
 
 extern crate getopts;
 extern crate rustc;

src/changes.rs

@@ -875,7 +875,7 @@ impl<'tcx> ChangeSet<'tcx> {
     pub fn trait_item_breaking(&self, old: DefId) -> bool {
         self.changes
             .get(&old)
-            .map_or(false, |change| change.trait_item_breaking())
+            .map_or(false, Change::trait_item_breaking)
     }
 
     /// Format the contents of a change set for user output.

tests/cases/regions/stdout

@@ -31,5 +31,21 @@ error: breaking changes in `def`
    |
    = warning: type error: expected reference, found bool (breaking)
 
-error: aborting due to 4 previous errors
+error: breaking changes in `efg`
+  --> $REPO_PATH/tests/cases/regions/new.rs:17:1
+   |
+17 | pub fn efg(_: &str) { }
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = warning: type error: expected bound lifetime parameterBrAnon(0), found concrete lifetime (breaking)
+
+error: breaking changes in `fgh`
+  --> $REPO_PATH/tests/cases/regions/new.rs:19:1
+   |
+19 | pub fn fgh(_: &'static str) { }
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = warning: type error: expected bound lifetime parameterBrAnon(0), found concrete lifetime (breaking)
+
+error: aborting due to 6 previous errors
 

tests/examples.rs

@@ -5,9 +5,7 @@ mod features {
         process::{Command, Stdio},
     };
 
-    fn test_example(path: &Path, out_file: &Path) {
-        let mut success = true;
-
+    fn test_example(path: &Path, out_file: &Path, expected_result: bool) {
         let old_rlib = path.join("libold.rlib").to_str().unwrap().to_owned();
         let new_rlib = path.join("libnew.rlib").to_str().unwrap().to_owned();
 
@@ -29,8 +27,8 @@ mod features {
                 cmd.args(target_args);
             }
 
-            success &= cmd.status().expect("could not run rustc").success();
-            assert!(success, "couldn't compile old");
+            let rustc_old_result = cmd.status().expect("could not run rustc on old").success();
+            assert!(rustc_old_result, "couldn't compile old");
 
             let mut cmd = Command::new("rustc");
             cmd.args(&["--crate-type=lib", "-o", &new_rlib])
@@ -42,9 +40,8 @@ mod features {
                 cmd.args(target_args);
             }
 
-            success &= cmd.status().expect("could not run rustc").success();
-
-            assert!(success, "couldn't compile new");
+            let rustc_new_result = cmd.status().expect("could not run rustc on new").success();
+            assert!(rustc_new_result, "couldn't compile new");
 
             let mut cmd = Command::new(
                 Path::new(".")
@@ -81,12 +78,14 @@ mod features {
                 cmd.env("RUST_SEMVER_API_GUIDELINES", "true");
             }
 
-            success &= cmd
+            let rustsemverver_result = cmd
                 .status()
                 .expect("could not run rust-semverver")
                 .success();
-
-            assert!(success, "rust-semverver");
+            assert_eq!(
+                rustsemverver_result, expected_result,
+                "rust-semverver returned an unexpected exit status"
+            );
 
             {
                 // replace root path with with $REPO_PATH
@@ -122,7 +121,7 @@ mod features {
             }
         }
 
-        success &= Command::new("git")
+        let git_result = Command::new("git")
             .args(&[
                 "diff",
                 "--ignore-space-at-eol",
@@ -134,7 +133,7 @@ mod features {
             .expect("could not run git diff")
             .success();
 
-        assert!(success, "git");
+        assert!(git_result, "git reports unexpected diff");
 
         Command::new("rm")
             .args(&[&old_rlib, &new_rlib])
@@ -143,54 +142,57 @@ mod features {
     }
 
     macro_rules! test {
-        ($name:ident) => {
+        ($name:ident => $result:literal) => {
             #[test]
             fn $name() {
                 let path = Path::new("tests").join("cases").join(stringify!($name));
-                test_example(&path, &path.join("stdout"));
+                test_example(&path, &path.join("stdout"), $result);
 
                 if path.join("stdout_api_guidelines").exists() {
                     eprintln!("api-guidelines");
-                    test_example(&path, &path.join("stdout_api_guidelines"));
+                    test_example(&path, &path.join("stdout_api_guidelines"), $result);
                 }
             }
         };
-        ($($name:ident),*) => {
-            $(test!($name);)*
-        }
+        ($($name:ident => $result:literal),*) => {
+            $(test!($name => $result);)*
+        };
+        ($($name:ident => $result:literal,)*) => {
+            $(test!($name => $result);)*
+        };
     }
 
     test! {
-        addition,
-        addition_path,
-        addition_use,
-        bounds,
-        circular,
-        consts,
-        enums,
-        func,
-        func_local_items,
-        infer,
-        infer_regress,
-        inherent_impls,
-        issue_34,
-        issue_50,
-        kind_change,
-        macros,
-        max_priv,
-        mix,
-        pathologic_paths,
-        pub_use,
-        regions,
-        removal,
-        removal_path,
-        removal_use,
-        sealed_traits,
-        structs,
-        swap,
-        traits,
-        trait_impls,
-        trait_objects,
-        ty_alias
+        addition => true,
+        addition_path => true,
+        addition_use => false,
+        bounds => false,
+        circular => true,
+        consts => false,
+        enums => false,
+        func => false,
+        func_local_items => true,
+        infer => true,
+        infer_regress => false,
+        inherent_impls => false,
+        issue_34 => true,
+        issue_50 => true,
+        kind_change => false,
+        macros => false,
+        max_priv => true,
+        mix => false,
+        pathologic_paths => true,
+        pub_use => true,
+        regions => false,
+        removal => false,
+        removal_path => false,
+        removal_use => false,
+        sealed_traits => true,
+        structs => false,
+        swap => true,
+        traits => false,
+        trait_impls => false,
+        trait_objects => true,
+        ty_alias => false,
     }
 }