Skip to content

Commit ee3e071

Browse files
authored
Merge pull request #339 from ehuss/regress-on-defaults
Add default regression terms based on `--regress` option.
2 parents a05c97c + 948bd7f commit ee3e071

File tree

8 files changed

+61
-57
lines changed

8 files changed

+61
-57
lines changed

Diff for: guide/src/examples/diagnostics.md

+4-2
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ fi
2323
echo "$OUTPUT"
2424
# This indicates a regression when the text "non-ASCII" is in the output.
2525
#
26-
# If the regression is when the text is *not* in the output, remove the `!` prefix.
26+
# If the regression is when the text is *not* in the output, remove the `!` prefix
27+
# (and customize the `--term-old` and `--term-new` CLI options if you want).
2728
! echo "$OUTPUT" | grep "non-ASCII"
2829
```
2930

3031
Then run something like:
3132

3233
```sh
33-
cargo bisect-rustc --start=1.67.0 --end=1.68.0 --script ./test.sh
34+
cargo bisect-rustc --start=1.67.0 --end=1.68.0 --script ./test.sh \
35+
--term-old="No warning" --term-new="Found non-ASCII warning"
3436
```

Diff for: guide/src/examples/doc-change.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ fi
2727
And run with:
2828

2929
```sh
30-
cargo bisect-rustc --start 1.68.0 --end 1.69.0 -c rust-docs --script ./test.sh
30+
cargo bisect-rustc --start 1.68.0 --end 1.69.0 -c rust-docs --script ./test.sh \
31+
--term-old="Did not find" --term-new="Found"
3132
```
3233

3334
> **Note**: This may not work on all targets since `cargo-bisect-rustc` doesn't properly handle rustup manifests, which alias some targets to other targets.

Diff for: guide/src/examples/rustdoc.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,6 @@ grep "some example text" $CARGO_TARGET_DIR/doc/mycrate/fn.foo.html
2727
This can be used with the `--script` option:
2828

2929
```sh
30-
cargo-bisect-rustc --start=2023-01-22 --end=2023-03-18 --script=./test.sh
30+
cargo-bisect-rustc --start=2023-01-22 --end=2023-03-18 --script=./test.sh \
31+
--term-old="Found example text" --term-new="Failed, or did not find text"
3132
```

Diff for: guide/src/tutorial.md

+4-14
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ cargo bisect-rustc --script=./test.sh \
138138

139139
## Custom bisection messages
140140

141-
[Available from v0.6.9]
141+
*Available from v0.6.9*
142142

143143
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.
144144

@@ -151,18 +151,8 @@ cargo bisect-rustc \
151151
--term-new "Yes, this build reproduces the regression, compile error"
152152
```
153153

154-
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.
154+
In other words, `--term-old` is displayed for older compilers that **do not** exhibit the regression. `--term-new` is for newer compilers which do exhibit the regression.
155155

156-
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.
156+
What counts as a "regression" is defined by the [`--regress`](usage.html#regression-check) CLI option. By default, a regression is a compile-error (which is equivalent to `--term-new`). If you flip the definition of a "regression" with `--regress=success`, then a regression is a successful compile (which is *also* equivalent to `--term-new`).
157157

158-
Example:
159-
```sh
160-
cargo bisect-rustc \
161-
--start=2018-08-14 \
162-
--end=2018-10-11 \
163-
--regress=success \
164-
--term-old "This build does not compile" \
165-
--term-new "This build compiles"
166-
```
167-
168-
See [`--regress`](usage.html#regression-check) for more details.
158+
There are default terms based on the current `--regress` setting. Customizing the terms is most useful when using [scripting](#testing-with-a-script). For example, in the [Documentation changes](examples/doc-change.md) example, the customized terms can more clearly express the results of the script of whether or not it found what it was looking for in the documentation.

Diff for: src/least_satisfying.rs

+4-22
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
use std::collections::BTreeMap;
22
use std::fmt;
33

4-
use crate::RegressOn;
5-
64
pub fn least_satisfying<T, P>(slice: &[T], mut predicate: P) -> usize
75
where
86
T: fmt::Display + fmt::Debug,
@@ -175,27 +173,11 @@ pub enum Satisfies {
175173
}
176174

177175
impl Satisfies {
178-
pub fn msg_with_context(&self, regress: &RegressOn, term_old: &str, term_new: &str) -> String {
179-
let msg_yes = if regress == &RegressOn::Error || regress == &RegressOn::Ice {
180-
// YES: compiles, does not reproduce the regression
181-
term_new
182-
} else {
183-
// NO: compile error, reproduces the regression
184-
term_old
185-
};
186-
187-
let msg_no = if regress == &RegressOn::Error || regress == &RegressOn::Ice {
188-
// YES: compile error
189-
term_old
190-
} else {
191-
// NO: compiles
192-
term_new
193-
};
194-
176+
pub fn msg_with_context<'a>(&self, term_old: &'a str, term_new: &'a str) -> &'a str {
195177
match self {
196-
Self::Yes => msg_yes.to_string(),
197-
Self::No => msg_no.to_string(),
198-
Self::Unknown => "Unable to figure out if the condition matched".to_string(),
178+
Self::Yes => term_new,
179+
Self::No => term_old,
180+
Self::Unknown => "Unable to figure out if the condition matched",
199181
}
200182
}
201183
}

Diff for: src/main.rs

+41-7
Original file line numberDiff line numberDiff line change
@@ -193,15 +193,13 @@ a date (YYYY-MM-DD), git tag name (e.g. 1.58.0) or git commit SHA."
193193

194194
#[arg(
195195
long,
196-
default_value = "Test condition NOT matched",
197-
help = "Text shown when a test fails to match the condition requested"
196+
help = "Text shown when a test does match the condition requested"
198197
)]
199198
term_new: Option<String>,
200199

201200
#[arg(
202201
long,
203-
default_value = "Test condition matched",
204-
help = "Text shown when a test does match the condition requested"
202+
help = "Text shown when a test fails to match the condition requested"
205203
)]
206204
term_old: Option<String>,
207205
}
@@ -890,8 +888,44 @@ impl Config {
890888
dl_spec: &DownloadParams,
891889
) -> Result<Satisfies, InstallError> {
892890
let regress = self.args.regress;
893-
let term_old = self.args.term_old.clone().unwrap_or_default();
894-
let term_new = self.args.term_new.clone().unwrap_or_default();
891+
let term_old = self.args.term_old.as_deref().unwrap_or_else(|| {
892+
if self.args.script.is_some() {
893+
match regress {
894+
RegressOn::Error => "Script returned success",
895+
RegressOn::Success => "Script returned error",
896+
RegressOn::Ice => "Script did not ICE",
897+
RegressOn::NonIce => "Script found ICE",
898+
RegressOn::NonError => "Script returned error (no ICE)",
899+
}
900+
} else {
901+
match regress {
902+
RegressOn::Error => "Successfully compiled",
903+
RegressOn::Success => "Compile error",
904+
RegressOn::Ice => "Did not ICE",
905+
RegressOn::NonIce => "Found ICE",
906+
RegressOn::NonError => "Compile error (no ICE)",
907+
}
908+
}
909+
});
910+
let term_new = self.args.term_new.as_deref().unwrap_or_else(|| {
911+
if self.args.script.is_some() {
912+
match regress {
913+
RegressOn::Error => "Script returned error",
914+
RegressOn::Success => "Script returned success",
915+
RegressOn::Ice => "Script found ICE",
916+
RegressOn::NonIce => "Script did not ICE",
917+
RegressOn::NonError => "Script returned success or ICE",
918+
}
919+
} else {
920+
match regress {
921+
RegressOn::Error => "Compile error",
922+
RegressOn::Success => "Successfully compiled",
923+
RegressOn::Ice => "Found ICE",
924+
RegressOn::NonIce => "Did not ICE",
925+
RegressOn::NonError => "Successfully compiled or ICE",
926+
}
927+
}
928+
});
895929
match t.install(&self.client, dl_spec) {
896930
Ok(()) => {
897931
let outcome = t.test(self);
@@ -903,7 +937,7 @@ impl Config {
903937
eprintln!(
904938
"RESULT: {}, ===> {}",
905939
t,
906-
r.msg_with_context(&regress, &term_old, &term_new)
940+
r.msg_with_context(term_old, term_new)
907941
);
908942
remove_toolchain(self, t, dl_spec);
909943
eprintln!();

Diff for: tests/cmd/h.stdout

+2-4
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,8 @@ Options:
2828
-t, --timeout <TIMEOUT> Assume failure after specified number of seconds (for bisecting
2929
hangs)
3030
--target <TARGET> Cross-compilation target platform
31-
--term-new <TERM_NEW> Text shown when a test fails to match the condition requested
32-
[default: "Test condition NOT matched"]
33-
--term-old <TERM_OLD> Text shown when a test does match the condition requested [default:
34-
"Test condition matched"]
31+
--term-new <TERM_NEW> Text shown when a test does match the condition requested
32+
--term-old <TERM_OLD> Text shown when a test fails to match the condition requested
3533
--test-dir <TEST_DIR> Root directory for tests [default: .]
3634
-v, --verbose...
3735
-V, --version Print version

Diff for: tests/cmd/help.stdout

+2-6
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,10 @@ Options:
9292
Cross-compilation target platform
9393

9494
--term-new <TERM_NEW>
95-
Text shown when a test fails to match the condition requested
96-
97-
[default: "Test condition NOT matched"]
95+
Text shown when a test does match the condition requested
9896

9997
--term-old <TERM_OLD>
100-
Text shown when a test does match the condition requested
101-
102-
[default: "Test condition matched"]
98+
Text shown when a test fails to match the condition requested
10399

104100
--test-dir <TEST_DIR>
105101
Root directory for tests

0 commit comments

Comments
 (0)