Skip to content

Commit bf10871

Browse files
committed
Fix flakiness in close_output test
It looks like stdout/stderr can race as to which gets printed first, but both are valid for this test. Closes #8665
1 parent d6b5b53 commit bf10871

File tree

2 files changed

+49
-54
lines changed

2 files changed

+49
-54
lines changed

crates/cargo-test-support/src/lib.rs

+40-38
Original file line numberDiff line numberDiff line change
@@ -1213,44 +1213,7 @@ impl Execs {
12131213
Ok(())
12141214
}
12151215
}
1216-
MatchKind::Unordered => {
1217-
let mut a = actual.lines().collect::<Vec<_>>();
1218-
// match more-constrained lines first, although in theory we'll
1219-
// need some sort of recursive match here. This handles the case
1220-
// that you expect "a\n[..]b" and two lines are printed out,
1221-
// "ab\n"a", where technically we do match unordered but a naive
1222-
// search fails to find this. This simple sort at least gets the
1223-
// test suite to pass for now, but we may need to get more fancy
1224-
// if tests start failing again.
1225-
a.sort_by_key(|s| s.len());
1226-
let mut failures = Vec::new();
1227-
1228-
for e_line in out.lines() {
1229-
match a.iter().position(|a_line| lines_match(e_line, a_line)) {
1230-
Some(index) => {
1231-
a.remove(index);
1232-
}
1233-
None => failures.push(e_line),
1234-
}
1235-
}
1236-
if !failures.is_empty() {
1237-
return Err(format!(
1238-
"Did not find expected line(s):\n{}\n\
1239-
Remaining available output:\n{}\n",
1240-
failures.join("\n"),
1241-
a.join("\n")
1242-
));
1243-
}
1244-
if !a.is_empty() {
1245-
Err(format!(
1246-
"Output included extra lines:\n\
1247-
{}\n",
1248-
a.join("\n")
1249-
))
1250-
} else {
1251-
Ok(())
1252-
}
1253-
}
1216+
MatchKind::Unordered => lines_match_unordered(&out, &actual),
12541217
}
12551218
}
12561219

@@ -1382,6 +1345,45 @@ pub fn lines_match(expected: &str, mut actual: &str) -> bool {
13821345
actual.is_empty() || expected.ends_with("[..]")
13831346
}
13841347

1348+
pub fn lines_match_unordered(expected: &str, actual: &str) -> Result<(), String> {
1349+
let mut a = actual.lines().collect::<Vec<_>>();
1350+
// match more-constrained lines first, although in theory we'll
1351+
// need some sort of recursive match here. This handles the case
1352+
// that you expect "a\n[..]b" and two lines are printed out,
1353+
// "ab\n"a", where technically we do match unordered but a naive
1354+
// search fails to find this. This simple sort at least gets the
1355+
// test suite to pass for now, but we may need to get more fancy
1356+
// if tests start failing again.
1357+
a.sort_by_key(|s| s.len());
1358+
let mut failures = Vec::new();
1359+
1360+
for e_line in expected.lines() {
1361+
match a.iter().position(|a_line| lines_match(e_line, a_line)) {
1362+
Some(index) => {
1363+
a.remove(index);
1364+
}
1365+
None => failures.push(e_line),
1366+
}
1367+
}
1368+
if !failures.is_empty() {
1369+
return Err(format!(
1370+
"Did not find expected line(s):\n{}\n\
1371+
Remaining available output:\n{}\n",
1372+
failures.join("\n"),
1373+
a.join("\n")
1374+
));
1375+
}
1376+
if !a.is_empty() {
1377+
Err(format!(
1378+
"Output included extra lines:\n\
1379+
{}\n",
1380+
a.join("\n")
1381+
))
1382+
} else {
1383+
Ok(())
1384+
}
1385+
}
1386+
13851387
/// Variant of `lines_match` that applies normalization to the strings.
13861388
pub fn normalized_lines_match(expected: &str, actual: &str, cwd: Option<&Path>) -> bool {
13871389
let expected = normalize_matcher(expected, cwd);

tests/testsuite/build.rs

+9-16
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use cargo::{
1010
use cargo_test_support::paths::{root, CargoPathExt};
1111
use cargo_test_support::registry::Package;
1212
use cargo_test_support::{
13-
basic_bin_manifest, basic_lib_manifest, basic_manifest, is_nightly, lines_match, main_file,
14-
paths, project, rustc_host, sleep_ms, symlink_supported, t, Execs, ProjectBuilder,
13+
basic_bin_manifest, basic_lib_manifest, basic_manifest, is_nightly, lines_match_unordered,
14+
main_file, paths, project, rustc_host, sleep_ms, symlink_supported, t, Execs, ProjectBuilder,
1515
};
1616
use std::env;
1717
use std::fs;
@@ -5040,29 +5040,22 @@ fn close_output() {
50405040
};
50415041

50425042
let stderr = spawn(false);
5043-
assert!(
5044-
lines_match(
5045-
"\
5043+
lines_match_unordered(
5044+
"\
50465045
[COMPILING] foo [..]
50475046
hello stderr!
50485047
[ERROR] [..]
50495048
[WARNING] build failed, waiting for other jobs to finish...
5050-
[ERROR] build failed
5049+
[ERROR] [..]
50515050
",
5052-
&stderr,
5053-
),
5054-
"lines differ:\n{}",
5055-
stderr
5056-
);
5051+
&stderr,
5052+
)
5053+
.unwrap();
50575054

50585055
// Try again with stderr.
50595056
p.build_dir().rm_rf();
50605057
let stdout = spawn(true);
5061-
assert!(
5062-
lines_match("hello stdout!\n", &stdout),
5063-
"lines differ:\n{}",
5064-
stdout
5065-
);
5058+
lines_match_unordered("hello stdout!\n", &stdout).unwrap();
50665059
}
50675060

50685061
use cargo_test_support::registry::Dependency;

0 commit comments

Comments
 (0)