Skip to content

Commit 00808b0

Browse files
committed
report: categorize spurious failures
1 parent d52ce05 commit 00808b0

File tree

6 files changed

+95
-50
lines changed

6 files changed

+95
-50
lines changed

src/report/html.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ impl ResultColor for Comparison {
2929
Comparison::SameTestSkipped => Color::Striped("#72a156", "#80b65f"),
3030
Comparison::SameTestPass => Color::Single("#72a156"),
3131
Comparison::Error => Color::Single("#d77026"),
32+
Comparison::SpuriousRegressed => Color::Striped("#db3026", "#d5433b"),
33+
Comparison::SpuriousFixed => Color::Striped("#5630db", "#5d3dcf"),
3234
}
3335
}
3436
}

src/report/mod.rs

Lines changed: 64 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -46,25 +46,29 @@ struct CrateResult {
4646
runs: [Option<BuildTestResult>; 2],
4747
}
4848

49-
#[derive(Serialize, Deserialize, Eq, PartialEq, Hash, Copy, Clone, Debug)]
50-
enum Comparison {
51-
Regressed,
52-
Fixed,
53-
Skipped,
54-
Unknown,
55-
Error,
56-
SameBuildFail,
57-
SameTestFail,
58-
SameTestSkipped,
59-
SameTestPass,
60-
}
49+
string_enum!(enum Comparison {
50+
Regressed => "regressed",
51+
Fixed => "fixed",
52+
Skipped => "skipped",
53+
Unknown => "unknown",
54+
Error => "error",
55+
SameBuildFail => "build-fail",
56+
SameTestFail => "test-fail",
57+
SameTestSkipped => "test-skipped",
58+
SameTestPass => "test-pass",
59+
SpuriousRegressed => "spurious-regressed",
60+
SpuriousFixed => "spurious-fixed",
61+
});
6162

6263
impl Comparison {
6364
fn show_in_summary(self) -> bool {
6465
match self {
65-
Comparison::Regressed | Comparison::Fixed | Comparison::Unknown | Comparison::Error => {
66-
true
67-
}
66+
Comparison::Regressed
67+
| Comparison::Fixed
68+
| Comparison::Unknown
69+
| Comparison::Error
70+
| Comparison::SpuriousRegressed
71+
| Comparison::SpuriousFixed => true,
6872
Comparison::Skipped
6973
| Comparison::SameBuildFail
7074
| Comparison::SameTestFail
@@ -74,26 +78,6 @@ impl Comparison {
7478
}
7579
}
7680

77-
impl Display for Comparison {
78-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
79-
write!(
80-
f,
81-
"{}",
82-
match self {
83-
Comparison::Regressed => "regressed",
84-
Comparison::Fixed => "fixed",
85-
Comparison::Skipped => "skipped",
86-
Comparison::Unknown => "unknown",
87-
Comparison::Error => "error",
88-
Comparison::SameBuildFail => "build-fail",
89-
Comparison::SameTestFail => "test-fail",
90-
Comparison::SameTestSkipped => "test-skipped",
91-
Comparison::SameTestPass => "test-pass",
92-
}
93-
)
94-
}
95-
}
96-
9781
#[derive(Serialize, Deserialize, Clone)]
9882
struct BuildTestResult {
9983
res: TestResult,
@@ -294,20 +278,49 @@ fn compare(
294278
r2: Option<TestResult>,
295279
) -> Comparison {
296280
use results::TestResult::*;
281+
297282
match (r1, r2) {
298283
(Some(res1), Some(res2)) => match (res1, res2) {
299284
(BuildFail(_), BuildFail(_)) => Comparison::SameBuildFail,
300285
(TestFail(_), TestFail(_)) => Comparison::SameTestFail,
301286
(TestSkipped, TestSkipped) => Comparison::SameTestSkipped,
302287
(TestPass, TestPass) => Comparison::SameTestPass,
288+
289+
(BuildFail(reason1), TestFail(reason2))
290+
if reason1.is_spurious() || reason2.is_spurious() =>
291+
{
292+
Comparison::SpuriousFixed
293+
}
294+
(BuildFail(reason), TestSkipped)
295+
| (BuildFail(reason), TestPass)
296+
| (TestFail(reason), TestPass)
297+
if reason.is_spurious() =>
298+
{
299+
Comparison::SpuriousFixed
300+
}
303301
(BuildFail(_), TestFail(_))
304302
| (BuildFail(_), TestSkipped)
305303
| (BuildFail(_), TestPass)
306304
| (TestFail(_), TestPass) => Comparison::Fixed,
305+
306+
(TestFail(reason1), BuildFail(reason2))
307+
if reason1.is_spurious() || reason2.is_spurious() =>
308+
{
309+
Comparison::SpuriousRegressed
310+
}
311+
(TestPass, TestFail(reason))
312+
| (TestPass, BuildFail(reason))
313+
| (TestSkipped, BuildFail(reason))
314+
| (TestFail(_), BuildFail(reason))
315+
if reason.is_spurious() =>
316+
{
317+
Comparison::SpuriousRegressed
318+
}
307319
(TestPass, TestFail(_))
308320
| (TestPass, BuildFail(_))
309321
| (TestSkipped, BuildFail(_))
310322
| (TestFail(_), BuildFail(_)) => Comparison::Regressed,
323+
311324
(Error, _) | (_, Error) => Comparison::Error,
312325
(TestFail(_), TestSkipped)
313326
| (TestPass, TestSkipped)
@@ -548,6 +561,8 @@ mod tests {
548561
TestFail(Unknown), TestFail(Unknown) => SameTestFail;
549562
TestSkipped, TestSkipped => SameTestSkipped;
550563
TestPass, TestPass => SameTestPass;
564+
565+
// Non-spurious fixes/regressions
551566
BuildFail(Unknown), TestFail(Unknown) => Fixed;
552567
BuildFail(Unknown), TestSkipped => Fixed;
553568
BuildFail(Unknown), TestPass => Fixed;
@@ -556,6 +571,20 @@ mod tests {
556571
TestPass, BuildFail(Unknown) => Regressed;
557572
TestSkipped, BuildFail(Unknown) => Regressed;
558573
TestFail(Unknown), BuildFail(Unknown) => Regressed;
574+
575+
// Spurious fixes/regressions
576+
BuildFail(OOM), TestFail(Unknown) => SpuriousFixed;
577+
BuildFail(Unknown), TestFail(OOM) => SpuriousFixed;
578+
BuildFail(OOM), TestSkipped => SpuriousFixed;
579+
BuildFail(OOM), TestPass => SpuriousFixed;
580+
TestFail(OOM), TestPass => SpuriousFixed;
581+
TestPass, TestFail(OOM) => SpuriousRegressed;
582+
TestPass, BuildFail(OOM) => SpuriousRegressed;
583+
TestSkipped, BuildFail(OOM) => SpuriousRegressed;
584+
TestFail(OOM), BuildFail(Unknown) => SpuriousRegressed;
585+
TestFail(Unknown), BuildFail(OOM) => SpuriousRegressed;
586+
587+
// Errors
559588
Error, TestPass => Error;
560589
Error, TestSkipped => Error;
561590
Error, TestFail(Unknown) => Error;

src/results/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,15 @@ string_enum!(pub enum FailureReason {
115115
Timeout => "timeout",
116116
});
117117

118+
impl FailureReason {
119+
pub(crate) fn is_spurious(self) -> bool {
120+
match self {
121+
FailureReason::Unknown | FailureReason::Broken => false,
122+
FailureReason::OOM | FailureReason::Timeout => true,
123+
}
124+
}
125+
}
126+
118127
test_result_enum!(pub enum TestResult {
119128
with_reason {
120129
BuildFail(FailureReason) => "build-fail",

src/utils/macros.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
macro_rules! string_enum {
2-
(pub enum $name:ident { $($item:ident => $str:expr,)* }) => {
3-
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Copy, Clone)]
4-
pub enum $name {
2+
($vis:vis enum $name:ident { $($item:ident => $str:expr,)* }) => {
3+
#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone)]
4+
$vis enum $name {
55
$($item,)*
66
}
77

@@ -23,16 +23,20 @@ macro_rules! string_enum {
2323
}
2424

2525
impl $name {
26-
pub fn to_str(&self) -> &'static str {
26+
#[allow(dead_code)]
27+
$vis fn to_str(&self) -> &'static str {
2728
match *self {
2829
$($name::$item => $str,)*
2930
}
3031
}
3132

32-
pub fn possible_values() -> &'static [&'static str] {
33+
#[allow(dead_code)]
34+
$vis fn possible_values() -> &'static [&'static str] {
3335
&[$($str,)*]
3436
}
3537
}
38+
39+
impl_serde_from_parse!($name, expecting="foo");
3640
}
3741
}
3842

@@ -49,6 +53,7 @@ macro_rules! impl_serde_from_parse {
4953
}
5054

5155
fn visit_str<E: ::serde::de::Error>(self, input: &str) -> Result<$for, E> {
56+
use std::str::FromStr;
5257
$for::from_str(input).map_err(E::custom)
5358
}
5459
}

tests/minicrater/full/results.expected.json

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"crates": [
33
{
44
"name": "beta-fixed (local)",
5-
"res": "Fixed",
5+
"res": "fixed",
66
"runs": [
77
{
88
"log": "stable/local/beta-fixed",
@@ -17,7 +17,7 @@
1717
},
1818
{
1919
"name": "beta-regression (local)",
20-
"res": "Regressed",
20+
"res": "regressed",
2121
"runs": [
2222
{
2323
"log": "stable/local/beta-regression",
@@ -32,7 +32,7 @@
3232
},
3333
{
3434
"name": "broken-cargotoml (local)",
35-
"res": "SameBuildFail",
35+
"res": "build-fail",
3636
"runs": [
3737
{
3838
"log": "stable/local/broken-cargotoml",
@@ -47,7 +47,7 @@
4747
},
4848
{
4949
"name": "build-fail (local)",
50-
"res": "SameBuildFail",
50+
"res": "build-fail",
5151
"runs": [
5252
{
5353
"log": "stable/local/build-fail",
@@ -62,7 +62,7 @@
6262
},
6363
{
6464
"name": "build-pass (local)",
65-
"res": "SameTestPass",
65+
"res": "test-pass",
6666
"runs": [
6767
{
6868
"log": "stable/local/build-pass",
@@ -77,7 +77,7 @@
7777
},
7878
{
7979
"name": "memory-hungry (local)",
80-
"res": "Fixed",
80+
"res": "spurious-fixed",
8181
"runs": [
8282
{
8383
"log": "stable/local/memory-hungry",
@@ -92,7 +92,7 @@
9292
},
9393
{
9494
"name": "network-access (local)",
95-
"res": "Fixed",
95+
"res": "fixed",
9696
"runs": [
9797
{
9898
"log": "stable/local/network-access",
@@ -107,7 +107,7 @@
107107
},
108108
{
109109
"name": "test-fail (local)",
110-
"res": "SameTestFail",
110+
"res": "test-fail",
111111
"runs": [
112112
{
113113
"log": "stable/local/test-fail",

tests/minicrater/small/results.expected.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"crates": [
33
{
44
"name": "beta-regression (local)",
5-
"res": "Regressed",
5+
"res": "regressed",
66
"runs": [
77
{
88
"log": "stable/local/beta-regression",
@@ -17,7 +17,7 @@
1717
},
1818
{
1919
"name": "build-pass (local)",
20-
"res": "SameTestPass",
20+
"res": "test-pass",
2121
"runs": [
2222
{
2323
"log": "stable/local/build-pass",

0 commit comments

Comments
 (0)