Skip to content

Commit b73c839

Browse files
sunshowerssunshowers
authored
[nextest-runner] improve leaked-handles text strings (#3199)
We had a bunch of different ways to represent "and leaked handles". Replace all of them with "(leaked handles)", which is equal or better in all cases. Also fix a TODO around this and switch InfoResponse over to using the platform-independent description types.
1 parent b804b91 commit b73c839

File tree

8 files changed

+139
-79
lines changed

8 files changed

+139
-79
lines changed

‎integration-tests/tests/integration/fixtures.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,7 +1278,7 @@ fn verify_junit(expected: &ExpectedTestResults, junit_path: &Utf8Path, propertie
12781278
// Expected JUnit type strings. These match the strings produced by nextest in
12791279
// junit.rs. The Rust test harness always uses exit code 101 for test failures.
12801280
const JUNIT_FAIL: &str = "test failure with exit code 101";
1281-
const JUNIT_FAIL_LEAK: &str = "test failure with exit code 101, and leaked handles";
1281+
const JUNIT_FAIL_LEAK: &str = "test failure with exit code 101 (leaked handles)";
12821282
const JUNIT_FLAKY_FAIL: &str = "flaky failure";
12831283
const JUNIT_ABORT: &str = "test abort";
12841284

@@ -1383,7 +1383,7 @@ impl JunitExpectedMessage<'_> {
13831383
///
13841384
/// Expected formats from nextest-runner/src/reporter/aggregator/junit.rs:
13851385
/// - Fail (exit code, no leak): "test failure with exit code 101"
1386-
/// - Fail (exit code, leaked): "test failure with exit code 101, and leaked handles"
1386+
/// - Fail (exit code, leaked): "test failure with exit code 101 (leaked handles)"
13871387
/// - Abort (no leak): "test abort"
13881388
/// - LeakFail: "test exited with code 0, but leaked handles so was marked failed"
13891389
/// - Timeout: "test timeout" or "benchmark timeout"

‎nextest-runner/src/reporter/aggregator/junit.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ fn non_success_kind_and_type(
372372
} => (
373373
NonSuccessKind::Failure,
374374
format!(
375-
"{} abort with leaked handles",
375+
"{} abort (leaked handles)",
376376
DisplayUnitKind::new(mode, kind),
377377
),
378378
),
@@ -389,7 +389,7 @@ fn non_success_kind_and_type(
389389
} => (
390390
NonSuccessKind::Failure,
391391
format!(
392-
"{} failure with exit code {code}, and leaked handles",
392+
"{} failure with exit code {code} (leaked handles)",
393393
DisplayUnitKind::new(mode, kind),
394394
),
395395
),
@@ -727,7 +727,7 @@ mod tests {
727727
* error waiting for child process to exit
728728
caused by:
729729
- huh
730-
* process aborted with signal 15 (SIGTERM), and also leaked handles
730+
* process aborted with signal 15 (SIGTERM) (leaked handles)
731731
* thread 'foo' panicked at xyz.rs:40
732732
"}),
733733
system_out: None,

‎nextest-runner/src/reporter/displayer/imp.rs‎

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2039,9 +2039,8 @@ impl<'a> DisplayReporterImpl<'a> {
20392039
DisplayUnitKind::new(self.mode, kind)
20402040
)?;
20412041

2042-
let tentative_desc = tentative_result.map(ExecutionResultDescription::from);
20432042
self.write_info_execution_result(
2044-
tentative_desc.as_ref(),
2043+
tentative_result.as_ref(),
20452044
slow_after.is_some(),
20462045
writer,
20472046
)?;
@@ -2083,8 +2082,7 @@ impl<'a> DisplayReporterImpl<'a> {
20832082
"{status_str}: {attempt_str}{} ",
20842083
DisplayUnitKind::new(self.mode, kind)
20852084
)?;
2086-
let result_desc = ExecutionResultDescription::from(*result);
2087-
self.write_info_execution_result(Some(&result_desc), slow_after.is_some(), writer)?;
2085+
self.write_info_execution_result(Some(result), slow_after.is_some(), writer)?;
20882086
write!(writer, " after {:.3?}s", time_taken.as_secs_f64())?;
20892087
if let Some(slow_after) = slow_after {
20902088
write!(
@@ -2106,8 +2104,7 @@ impl<'a> DisplayReporterImpl<'a> {
21062104
"{status_str}: {attempt_str}{} ",
21072105
DisplayUnitKind::new(self.mode, kind)
21082106
)?;
2109-
let previous_desc = ExecutionResultDescription::from(*previous_result);
2110-
self.write_info_execution_result(Some(&previous_desc), *previous_slow, writer)?;
2107+
self.write_info_execution_result(Some(previous_result), *previous_slow, writer)?;
21112108
writeln!(
21122109
writer,
21132110
", currently {} before next attempt",
@@ -2255,9 +2252,7 @@ impl<'a> DisplayReporterImpl<'a> {
22552252
}
22562253
Some(ExecutionResultDescription::Fail {
22572254
failure: FailureDescription::Abort { abort },
2258-
// TODO: show leaked info here like in FailureDescription::ExitCode
2259-
// below?
2260-
leaked: _,
2255+
leaked,
22612256
}) => {
22622257
// The errors are shown in the output.
22632258
write!(writer, "{}", "aborted".style(self.styles.fail))?;
@@ -2270,27 +2265,25 @@ impl<'a> DisplayReporterImpl<'a> {
22702265
write!(writer, ": SIG{s}")?;
22712266
}
22722267
}
2268+
if *leaked {
2269+
write!(writer, " (leaked handles)")?;
2270+
}
22732271
Ok(())
22742272
}
22752273
Some(ExecutionResultDescription::Fail {
22762274
failure: FailureDescription::ExitCode { code },
22772275
leaked,
22782276
}) => {
2277+
write!(
2278+
writer,
2279+
"{} with exit code {}",
2280+
"failed".style(self.styles.fail),
2281+
code.style(self.styles.count),
2282+
)?;
22792283
if *leaked {
2280-
write!(
2281-
writer,
2282-
"{} with exit code {}, and leaked handles",
2283-
"failed".style(self.styles.fail),
2284-
code.style(self.styles.count),
2285-
)
2286-
} else {
2287-
write!(
2288-
writer,
2289-
"{} with exit code {}",
2290-
"failed".style(self.styles.fail),
2291-
code.style(self.styles.count),
2292-
)
2284+
write!(writer, " (leaked handles)")?;
22932285
}
2286+
Ok(())
22942287
}
22952288
Some(ExecutionResultDescription::ExecFail) => {
22962289
write!(writer, "{}", "failed to execute".style(self.styles.fail))
@@ -3698,6 +3691,7 @@ mod tests {
36983691
let test_name2 = TestCaseName::new("test2");
36993692
let test_name3 = TestCaseName::new("test3");
37003693
let test_name4 = TestCaseName::new("test4");
3694+
let test_name5 = TestCaseName::new("test5");
37013695

37023696
let mut out = String::new();
37033697

@@ -3743,7 +3737,7 @@ mod tests {
37433737
elapsed: Duration::ZERO,
37443738
kind: TestEventKind::InfoResponse {
37453739
index: 0,
3746-
total: 20,
3740+
total: 21,
37473741
// Technically, you won't get setup script and test responses in the
37483742
// same response, but it's easiest to test in this manner.
37493743
response: InfoResponse::SetupScript(SetupScriptInfoResponse {
@@ -3774,7 +3768,7 @@ mod tests {
37743768
elapsed: Duration::ZERO,
37753769
kind: TestEventKind::InfoResponse {
37763770
index: 1,
3777-
total: 20,
3771+
total: 21,
37783772
response: InfoResponse::SetupScript(SetupScriptInfoResponse {
37793773
stress_index: None,
37803774
script_id: ScriptId::new(SmolStr::new("setup-slow")).unwrap(),
@@ -3804,7 +3798,7 @@ mod tests {
38043798
elapsed: Duration::ZERO,
38053799
kind: TestEventKind::InfoResponse {
38063800
index: 2,
3807-
total: 20,
3801+
total: 21,
38083802
response: InfoResponse::SetupScript(SetupScriptInfoResponse {
38093803
stress_index: None,
38103804
script_id: ScriptId::new(SmolStr::new("setup-terminating"))
@@ -3846,7 +3840,7 @@ mod tests {
38463840
elapsed: Duration::ZERO,
38473841
kind: TestEventKind::InfoResponse {
38483842
index: 3,
3849-
total: 20,
3843+
total: 21,
38503844
response: InfoResponse::SetupScript(SetupScriptInfoResponse {
38513845
stress_index: Some(StressIndex {
38523846
current: 0,
@@ -3862,7 +3856,7 @@ mod tests {
38623856
// Even if exit_status is 0, the presence of
38633857
// exec-fail errors should be considered
38643858
// part of the output.
3865-
tentative_result: Some(ExecutionResult::ExecFail),
3859+
tentative_result: Some(ExecutionResultDescription::ExecFail),
38663860
waiting_duration: Duration::from_millis(10467),
38673861
remaining: Duration::from_millis(335),
38683862
},
@@ -3882,7 +3876,7 @@ mod tests {
38823876
elapsed: Duration::ZERO,
38833877
kind: TestEventKind::InfoResponse {
38843878
index: 4,
3885-
total: 20,
3879+
total: 21,
38863880
response: InfoResponse::SetupScript(SetupScriptInfoResponse {
38873881
stress_index: Some(StressIndex {
38883882
current: 1,
@@ -3892,8 +3886,8 @@ mod tests {
38923886
program: "setup-exited".to_owned(),
38933887
args: args.clone(),
38943888
state: UnitState::Exited {
3895-
result: ExecutionResult::Fail {
3896-
failure_status: FailureStatus::ExitCode(1),
3889+
result: ExecutionResultDescription::Fail {
3890+
failure: FailureDescription::ExitCode { code: 1 },
38973891
leaked: true,
38983892
},
38993893
time_taken: Duration::from_millis(9999),
@@ -3915,7 +3909,7 @@ mod tests {
39153909
elapsed: Duration::ZERO,
39163910
kind: TestEventKind::InfoResponse {
39173911
index: 5,
3918-
total: 20,
3912+
total: 21,
39193913
response: InfoResponse::Test(TestInfoResponse {
39203914
stress_index: None,
39213915
test_instance: TestInstanceId {
@@ -3944,7 +3938,7 @@ mod tests {
39443938
elapsed: Duration::ZERO,
39453939
kind: TestEventKind::InfoResponse {
39463940
index: 6,
3947-
total: 20,
3941+
total: 21,
39483942
response: InfoResponse::Test(TestInfoResponse {
39493943
stress_index: Some(StressIndex {
39503944
current: 0,
@@ -3979,7 +3973,7 @@ mod tests {
39793973
elapsed: Duration::ZERO,
39803974
kind: TestEventKind::InfoResponse {
39813975
index: 7,
3982-
total: 20,
3976+
total: 21,
39833977
response: InfoResponse::Test(TestInfoResponse {
39843978
stress_index: None,
39853979
test_instance: TestInstanceId {
@@ -4011,7 +4005,7 @@ mod tests {
40114005
elapsed: Duration::ZERO,
40124006
kind: TestEventKind::InfoResponse {
40134007
index: 8,
4014-
total: 20,
4008+
total: 21,
40154009
response: InfoResponse::Test(TestInfoResponse {
40164010
stress_index: Some(StressIndex {
40174011
current: 1,
@@ -4026,7 +4020,7 @@ mod tests {
40264020
total_attempts: 5,
40274021
},
40284022
state: UnitState::Exited {
4029-
result: ExecutionResult::Pass,
4023+
result: ExecutionResultDescription::Pass,
40304024
time_taken: Duration::from_millis(99999),
40314025
slow_after: Some(Duration::from_millis(33333)),
40324026
},
@@ -4049,7 +4043,7 @@ mod tests {
40494043
elapsed: Duration::ZERO,
40504044
kind: TestEventKind::InfoResponse {
40514045
index: 9,
4052-
total: 20,
4046+
total: 21,
40534047
response: InfoResponse::Test(TestInfoResponse {
40544048
stress_index: None,
40554049
test_instance: TestInstanceId {
@@ -4064,7 +4058,7 @@ mod tests {
40644058
total_attempts: 5,
40654059
},
40664060
state: UnitState::DelayBeforeNextAttempt {
4067-
previous_result: ExecutionResult::ExecFail,
4061+
previous_result: ExecutionResultDescription::ExecFail,
40684062
previous_slow: true,
40694063
waiting_duration: Duration::from_millis(1234),
40704064
remaining: Duration::from_millis(5678),
@@ -4085,6 +4079,43 @@ mod tests {
40854079
})
40864080
.unwrap();
40874081

4082+
// A test that was aborted by a signal and leaked handles.
4083+
reporter
4084+
.write_event(&TestEvent {
4085+
timestamp: Local::now().into(),
4086+
elapsed: Duration::ZERO,
4087+
kind: TestEventKind::InfoResponse {
4088+
index: 10,
4089+
total: 21,
4090+
response: InfoResponse::Test(TestInfoResponse {
4091+
stress_index: None,
4092+
test_instance: TestInstanceId {
4093+
binary_id: &binary_id,
4094+
test_name: &test_name5,
4095+
},
4096+
retry_data: RetryData {
4097+
attempt: 1,
4098+
total_attempts: 1,
4099+
},
4100+
state: UnitState::Exited {
4101+
result: ExecutionResultDescription::Fail {
4102+
failure: FailureDescription::Abort {
4103+
abort: AbortDescription::UnixSignal {
4104+
signal: 11,
4105+
name: Some("SEGV".into()),
4106+
},
4107+
},
4108+
leaked: true,
4109+
},
4110+
time_taken: Duration::from_millis(5678),
4111+
slow_after: None,
4112+
},
4113+
output: make_split_output(None, "segfault output", ""),
4114+
}),
4115+
},
4116+
})
4117+
.unwrap();
4118+
40884119
reporter
40894120
.write_event(&TestEvent {
40904121
timestamp: Local::now().into(),

0 commit comments

Comments
 (0)