Skip to content

Commit

Permalink
Add custom "Satisfies" terms
Browse files Browse the repository at this point in the history
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
  • Loading branch information
apiraino committed May 3, 2024
1 parent 0859f47 commit dee4e39
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
28 changes: 28 additions & 0 deletions src/least_satisfying.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)
Expand Down
25 changes: 23 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down

0 comments on commit dee4e39

Please sign in to comment.