Skip to content

Commit

Permalink
Add custom "Satisfies" terms
Browse files Browse the repository at this point in the history
Currently, cargo-bisect-rustc prints "Yes" or "No" on whether or not a
build satisfies the condition that it is looking for. However, this
terminology is either confusing or backwards depending on what you are
testing.

For example, you can use one of --regress options 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-good, will apply for Satisfy::Yes (condition requested is
matched)
--term-bad, 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 Apr 10, 2024
1 parent 1c1dbaa commit e66020b
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_new: &str, term_old: &str) -> String {
let msg_yes = if regress == &RegressOn::Error || regress == &RegressOn::Ice {
// YES: condition satisfied, reproduces the regression
term_old
} else {
// NO: condition not satisfied, does not reproduce the regression
term_new
};

let msg_no = if regress == &RegressOn::Error || regress == &RegressOn::Ice {
// YES: condition satisfied, compiles successfully, does not reproduce the regression
term_new
} else {
// NO: condition not satisfied, the regression still reproduces
term_old
};

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_new = self.args.term_new.clone().unwrap_or_default();
let term_old = self.args.term_old.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_new, &term_old)
);
remove_toolchain(self, t, dl_spec);
eprintln!();
Ok(r)
Expand Down

0 comments on commit e66020b

Please sign in to comment.