Merge pull request #330 from apiraino/custom-satisfy-terms

Add custom "Satisfies" terms

Author: ehuss

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## v0.6.9
+
+### Added
+- Flags `--term-old` and `--term-new` to allow custom messages when bisecting a regression.
+
 ## v0.6.8
 
 ### Added

README.md

@@ -6,6 +6,14 @@ This tool bisects either Rust nightlies or CI artifacts.
 
 [**Documentation**](https://rust-lang.github.io/cargo-bisect-rustc/)
 
+To run the documentation book locally, install [mdBook](https://github.com/rust-lang/mdBook):
+
+``` sh
+cd guide
+mdbook serve # launch a local server to allow you to easily see and update changes you make
+mdbook build # build the book HTML
+```
+
 ## License
 
 Licensed under either of

guide/src/tutorial.md

@@ -135,3 +135,34 @@ cargo bisect-rustc --script=./test.sh \
 
 [issue #53157]: https://github.com/rust-lang/rust/issues/53157
 [issue #55036]: https://github.com/rust-lang/rust/issues/55036
+
+## Custom bisection messages
+
+[Available from v0.6.9]
+
+You can add custom messages when bisecting a regression. Taking inspiration from git-bisect, with `term-new` and `term-old` you can set custom messages to indicate if a regression matches the condition set by the bisection.
+
+Example:
+```sh
+cargo bisect-rustc \
+    --start=2018-08-14 \
+    --end=2018-10-11 \
+    --term-old "No, this build did not reproduce the regression, compile successful" \
+    --term-new "Yes, this build reproduces the regression, compile error"
+```
+
+Note that `--term-{old,new}` are aware of the `--regress` parameter. If the bisection is looking for a build to reproduce a regression (i.e. `--regress {error,ice}`), `--term-old` indicates a point in time where the regression does not reproduce and `--term-new` indicates that it does.
+
+On the other hand, if `--regress {non-error,non-ice,success}` you are looking into bisecting when a condition of error stopped being reproducible (e.g. some faulty code does not produce an error anymore). In this case `cargo-bisect` flips the meaning of these two parameters.
+
+Example:
+```sh
+cargo bisect-rustc \
+    --start=2018-08-14 \
+    --end=2018-10-11 \
+    --regress=success \
+    --term-old "This build does not compile" \
+    --term-new "This build compiles"
+```
+
+See [`--regress`](usage.html#regression-check) for more details.

src/least_satisfying.rs

@@ -1,6 +1,8 @@
 use std::collections::BTreeMap;
 use std::fmt;
 
+use crate::RegressOn;
+
 pub fn least_satisfying<T, P>(slice: &[T], mut predicate: P) -> usize
 where
     T: fmt::Display + fmt::Debug,
@@ -172,6 +174,32 @@ pub enum Satisfies {
     Unknown,
 }
 
+impl Satisfies {
+    pub fn msg_with_context(&self, regress: &RegressOn, term_old: &str, term_new: &str) -> String {
+        let msg_yes = if regress == &RegressOn::Error || regress == &RegressOn::Ice {
+            // YES: compiles, does not reproduce the regression
+            term_new
+        } else {
+            // NO: compile error, reproduces the regression
+            term_old
+        };
+
+        let msg_no = if regress == &RegressOn::Error || regress == &RegressOn::Ice {
+            // YES: compile error
+            term_old
+        } else {
+            // NO: compiles
+            term_new
+        };
+
+        match self {
+            Self::Yes => msg_yes.to_string(),
+            Self::No => msg_no.to_string(),
+            Self::Unknown => "Unable to figure out if the condition matched".to_string(),
+        }
+    }
+}
+
 impl fmt::Display for Satisfies {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{:?}", self)

src/main.rs

@@ -180,6 +180,20 @@ a date (YYYY-MM-DD), git tag name (e.g. 1.58.0) or git commit SHA."
 
     #[arg(long, help = "Do not install cargo [default: install cargo]")]
     without_cargo: bool,
+
+    #[arg(
+        long,
+        default_value = "Test condition NOT matched",
+        help = "Text shown when a test fails to match the condition requested"
+    )]
+    term_new: Option<String>,
+
+    #[arg(
+        long,
+        default_value = "Test condition matched",
+        help = "Text shown when a test does match the condition requested"
+    )]
+    term_old: Option<String>,
 }
 
 pub type GitDate = NaiveDate;
@@ -337,7 +351,7 @@ enum RegressOn {
     /// Marks test outcome as `Regressed` if and only if the `rustc`
     /// process issues a diagnostic indicating that an internal compiler error
     /// (ICE) occurred. This covers the use case for when you want to bisect to
-    /// see when an ICE was introduced pon a codebase that is meant to produce
+    /// see when an ICE was introduced on a codebase that is meant to produce
     /// a clean error.
     Ice,
 
@@ -865,6 +879,9 @@ impl Config {
         t: &Toolchain,
         dl_spec: &DownloadParams,
     ) -> Result<Satisfies, InstallError> {
+        let regress = self.args.regress;
+        let term_old = self.args.term_old.clone().unwrap_or_default();
+        let term_new = self.args.term_new.clone().unwrap_or_default();
         match t.install(&self.client, dl_spec) {
             Ok(()) => {
                 let outcome = t.test(self);
@@ -873,7 +890,11 @@ impl Config {
                     TestOutcome::Baseline => Satisfies::No,
                     TestOutcome::Regressed => Satisfies::Yes,
                 };
-                eprintln!("RESULT: {}, ===> {}", t, r);
+                eprintln!(
+                    "RESULT: {}, ===> {}",
+                    t,
+                    r.msg_with_context(&regress, &term_old, &term_new)
+                );
                 remove_toolchain(self, t, dl_spec);
                 eprintln!();
                 Ok(r)

tests/README.md

@@ -0,0 +1,18 @@
+# Quick guidelines for tests
+
+If you change the command line parameters of cargo-bisect, tests will fail, the crate `trycmd` is used to keep track of these changes.
+
+In order to update files under `tests/cmd/*.{stdout,stderr}`, run the test generating the new expected results:
+
+`TRYCMD=dump cargo test`
+
+it will create a `dump` directory in the project root. Then move `dump/*.{stdout,stderr}` into `./tests/cmd` and run tests again. They should be all green now.
+
+Note: if the local tests generate output specific for your machine, please replace that output with `[..]`, else CI tests will fail. Example:
+
+``` diff
+-      --host <HOST>             Host triple for the compiler [default: x86_64-unknown-linux-gnu]
++      --host <HOST>             Host triple for the compiler [default: [..]]
+```
+
+See the trycmd [documentation](https://docs.rs/trycmd/latest/trycmd/) for more info.

tests/cmd/h.stdout

@@ -15,7 +15,7 @@ Options:
                                 (YYYY-MM-DD), git tag name (e.g. 1.58.0) or git commit SHA.
       --force-install           Force installation over existing artifacts
   -h, --help                    Print help (see more with '--help')
-...
+      --host <HOST>             Host triple for the compiler [default: [..]]
       --install <INSTALL>       Install the given artifact
       --preserve                Preserve the downloaded artifacts
       --preserve-target         Preserve the target directory used for builds
@@ -28,6 +28,10 @@ Options:
   -t, --timeout <TIMEOUT>       Assume failure after specified number of seconds (for bisecting
                                 hangs)
       --target <TARGET>         Cross-compilation target platform
+      --term-new <TERM_NEW>     Text shown when a test fails to match the condition requested
+                                [default: "Test condition NOT matched"]
+      --term-old <TERM_OLD>     Text shown when a test does match the condition requested [default:
+                                "Test condition matched"]
       --test-dir <TEST_DIR>     Root directory for tests [default: .]
   -v, --verbose...              
   -V, --version                 Print version

tests/cmd/help.stdout

@@ -65,7 +65,7 @@ Options:
             fixed
           - ice:       Marks test outcome as `Regressed` if and only if the `rustc` process issues a
             diagnostic indicating that an internal compiler error (ICE) occurred. This covers the
-            use case for when you want to bisect to see when an ICE was introduced pon a codebase
+            use case for when you want to bisect to see when an ICE was introduced on a codebase
             that is meant to produce a clean error
           - non-ice:   Marks test outcome as `Regressed` if and only if the `rustc` process does not
             issue a diagnostic indicating that an internal compiler error (ICE) occurred. This
@@ -91,6 +91,16 @@ Options:
       --target <TARGET>
           Cross-compilation target platform
 
+      --term-new <TERM_NEW>
+          Text shown when a test fails to match the condition requested
+          
+          [default: "Test condition NOT matched"]
+
+      --term-old <TERM_OLD>
+          Text shown when a test does match the condition requested
+          
+          [default: "Test condition matched"]
+
       --test-dir <TEST_DIR>
           Root directory for tests