From bf10871c32ec2551c1b35e6728446ca655371b80 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 31 Aug 2020 13:15:02 -0700
Subject: [PATCH] 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
---
 crates/cargo-test-support/src/lib.rs | 78 ++++++++++++++--------------
 tests/testsuite/build.rs             | 25 ++++-----
 2 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs
index 5c64307425b..57da10dd54b 100644
--- a/crates/cargo-test-support/src/lib.rs
+++ b/crates/cargo-test-support/src/lib.rs
@@ -1213,44 +1213,7 @@ impl Execs {
                     Ok(())
                 }
             }
-            MatchKind::Unordered => {
-                let mut a = actual.lines().collect::<Vec<_>>();
-                // match more-constrained lines first, although in theory we'll
-                // need some sort of recursive match here. This handles the case
-                // that you expect "a\n[..]b" and two lines are printed out,
-                // "ab\n"a", where technically we do match unordered but a naive
-                // search fails to find this. This simple sort at least gets the
-                // test suite to pass for now, but we may need to get more fancy
-                // if tests start failing again.
-                a.sort_by_key(|s| s.len());
-                let mut failures = Vec::new();
-
-                for e_line in out.lines() {
-                    match a.iter().position(|a_line| lines_match(e_line, a_line)) {
-                        Some(index) => {
-                            a.remove(index);
-                        }
-                        None => failures.push(e_line),
-                    }
-                }
-                if !failures.is_empty() {
-                    return Err(format!(
-                        "Did not find expected line(s):\n{}\n\
-                         Remaining available output:\n{}\n",
-                        failures.join("\n"),
-                        a.join("\n")
-                    ));
-                }
-                if !a.is_empty() {
-                    Err(format!(
-                        "Output included extra lines:\n\
-                         {}\n",
-                        a.join("\n")
-                    ))
-                } else {
-                    Ok(())
-                }
-            }
+            MatchKind::Unordered => lines_match_unordered(&out, &actual),
         }
     }
 
@@ -1382,6 +1345,45 @@ pub fn lines_match(expected: &str, mut actual: &str) -> bool {
     actual.is_empty() || expected.ends_with("[..]")
 }
 
+pub fn lines_match_unordered(expected: &str, actual: &str) -> Result<(), String> {
+    let mut a = actual.lines().collect::<Vec<_>>();
+    // match more-constrained lines first, although in theory we'll
+    // need some sort of recursive match here. This handles the case
+    // that you expect "a\n[..]b" and two lines are printed out,
+    // "ab\n"a", where technically we do match unordered but a naive
+    // search fails to find this. This simple sort at least gets the
+    // test suite to pass for now, but we may need to get more fancy
+    // if tests start failing again.
+    a.sort_by_key(|s| s.len());
+    let mut failures = Vec::new();
+
+    for e_line in expected.lines() {
+        match a.iter().position(|a_line| lines_match(e_line, a_line)) {
+            Some(index) => {
+                a.remove(index);
+            }
+            None => failures.push(e_line),
+        }
+    }
+    if !failures.is_empty() {
+        return Err(format!(
+            "Did not find expected line(s):\n{}\n\
+                         Remaining available output:\n{}\n",
+            failures.join("\n"),
+            a.join("\n")
+        ));
+    }
+    if !a.is_empty() {
+        Err(format!(
+            "Output included extra lines:\n\
+                         {}\n",
+            a.join("\n")
+        ))
+    } else {
+        Ok(())
+    }
+}
+
 /// Variant of `lines_match` that applies normalization to the strings.
 pub fn normalized_lines_match(expected: &str, actual: &str, cwd: Option<&Path>) -> bool {
     let expected = normalize_matcher(expected, cwd);
diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs
index f4068ae3c19..40673506e66 100644
--- a/tests/testsuite/build.rs
+++ b/tests/testsuite/build.rs
@@ -10,8 +10,8 @@ use cargo::{
 use cargo_test_support::paths::{root, CargoPathExt};
 use cargo_test_support::registry::Package;
 use cargo_test_support::{
-    basic_bin_manifest, basic_lib_manifest, basic_manifest, is_nightly, lines_match, main_file,
-    paths, project, rustc_host, sleep_ms, symlink_supported, t, Execs, ProjectBuilder,
+    basic_bin_manifest, basic_lib_manifest, basic_manifest, is_nightly, lines_match_unordered,
+    main_file, paths, project, rustc_host, sleep_ms, symlink_supported, t, Execs, ProjectBuilder,
 };
 use std::env;
 use std::fs;
@@ -5040,29 +5040,22 @@ fn close_output() {
     };
 
     let stderr = spawn(false);
-    assert!(
-        lines_match(
-            "\
+    lines_match_unordered(
+        "\
 [COMPILING] foo [..]
 hello stderr!
 [ERROR] [..]
 [WARNING] build failed, waiting for other jobs to finish...
-[ERROR] build failed
+[ERROR] [..]
 ",
-            &stderr,
-        ),
-        "lines differ:\n{}",
-        stderr
-    );
+        &stderr,
+    )
+    .unwrap();
 
     // Try again with stderr.
     p.build_dir().rm_rf();
     let stdout = spawn(true);
-    assert!(
-        lines_match("hello stdout!\n", &stdout),
-        "lines differ:\n{}",
-        stdout
-    );
+    lines_match_unordered("hello stdout!\n", &stdout).unwrap();
 }
 
 use cargo_test_support::registry::Dependency;