From dee4e397d672961554cc85cc2140d76a2532577a Mon Sep 17 00:00:00 2001
From: apiraino <apiraino@users.noreply.github.com>
Date: Fri, 3 May 2024 17:54:56 +0200
Subject: [PATCH] Add custom "Satisfies" terms

cargo-bisect-rustc prints "Yes" or "No" on whether or not a build
satisfies the condition that it is looking for (default: Yes =
reproduces regression, No = does not reproduce the regression). However,
this terminology is either confusing or backwards depending on what is
being tested.

For example, one can use one of `--regress` options (f.e. `--regress
success`) to find when a regression was fixed. In that sense, the "old"
is "regression found" and the "new" is "regression fixed", which is
backwards from the normal behavior.

Taking inspiration from git bisect, we are introducing custom terms for
Satisfies. This is implemented with 2 new cli options:

--term-old, will apply for Satisfy::Yes (condition requested is matched)
--term-new, will apply for Satisfy::No (condition requested is NOT matched)

This will allow the user to specify their own wording. Then, the
--regress option could set the defaults for those terms appropriate for
the regression type.

Fixes #316
---
 src/least_satisfying.rs | 28 ++++++++++++++++++++++++++++
 src/main.rs             | 25 +++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/src/least_satisfying.rs b/src/least_satisfying.rs
index a25717d..1947746 100644
--- a/src/least_satisfying.rs
+++ b/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)
diff --git a/src/main.rs b/src/main.rs
index 8699039..c95bebe 100644
--- a/src/main.rs
+++ b/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)