From f42bf6f946012a09fc9e37c3af6b31bcd6c93e24 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Wed, 9 Dec 2020 13:02:23 -0600
Subject: [PATCH] Fix the feature tests with a retry (#8534)

A bunch of our feature tests don't work reliably in CI. They rely on
creating a new `OpenConsole.exe` window, then running the test _in that
console_. As a part of that test setup, the test runner used to wait a
second to attach to the newly created console. Then the test goes on
it's merry way, assuming the console is ready to go. However, in CI,
that might take more than a second. If it does, then the test would fail
pretty immediately, as soon as it tries to get at the buffer of the new
console.

This PR introduces a little retry loop to the test init. After attaching
to the new console, we'll try and get at the screen buffer. If that
fails, we'll wait a second and try again. We'll try 5 times total,
before bailing entirely. Hopefully, this should mitigate most of the
random CI failures we get in the feature tests.

Closes #8495
---
 src/host/ft_host/InitTests.cpp | 50 ++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/src/host/ft_host/InitTests.cpp b/src/host/ft_host/InitTests.cpp
index d6a556608ddb..ea738dd8fa3c 100644
--- a/src/host/ft_host/InitTests.cpp
+++ b/src/host/ft_host/InitTests.cpp
@@ -5,6 +5,7 @@
 
 using namespace WEX::TestExecution;
 using namespace WEX::Common;
+using namespace WEX::Logging;
 
 const DWORD _dwMaxMillisecondsToWaitOnStartup = 120 * 1000;
 const DWORD _dwStartupWaitPollingIntervalInMilliseconds = 200;
@@ -236,19 +237,46 @@ MODULE_SETUP(ModuleSetup)
 
     // Wait a moment for the driver to be ready after freeing to attach.
     Sleep(1000);
-
     VERIFY_WIN32_BOOL_SUCCEEDED_RETURN(AttachConsole(dwFindPid));
 
-    // Replace CRT handles
-    // These need to be reopened as read/write or they can affect some of the tests.
-    //
-    // std_out and std_in need to be closed when tests are finished, this is handled by the wil::scope_exit at the
-    // top of this file.
-    errno_t err = 0;
-    err = freopen_s(&std_out, "CONOUT$", "w+", stdout);
-    VERIFY_ARE_EQUAL(0, err);
-    err = freopen_s(&std_in, "CONIN$", "r+", stdin);
-    VERIFY_ARE_EQUAL(0, err);
+    int tries = 0;
+    while (tries < 5)
+    {
+        tries++;
+        Log::Comment(NoThrowString().Format(L"Attempt #%d to confirm we've attached", tries));
+
+        // Replace CRT handles
+        // These need to be reopened as read/write or they can affect some of the tests.
+        //
+        // std_out and std_in need to be closed when tests are finished, this is handled by the wil::scope_exit at the
+        // top of this file.
+        errno_t err = 0;
+        err = freopen_s(&std_out, "CONOUT$", "w+", stdout);
+        VERIFY_ARE_EQUAL(0, err);
+        err = freopen_s(&std_in, "CONIN$", "r+", stdin);
+        VERIFY_ARE_EQUAL(0, err);
+
+        // Now, try to get at the console we've set up. It's possible 1s wasn't long enough. If that was, we'll try again.
+
+        HANDLE const hOut = GetStdOutputHandle();
+        VERIFY_IS_NOT_NULL(hOut, L"Verify we have the standard output handle.");
+
+        CONSOLE_SCREEN_BUFFER_INFOEX csbiexBefore = { 0 };
+        csbiexBefore.cbSize = sizeof(csbiexBefore);
+        BOOL succeeded = GetConsoleScreenBufferInfoEx(hOut, &csbiexBefore);
+        if (!succeeded)
+        {
+            auto gle = GetLastError();
+            VERIFY_ARE_EQUAL(6u, gle, L"If we fail to set up the console, GetLastError should return 6 here.");
+            Sleep(1000);
+        }
+        else
+        {
+            break;
+        }
+    };
+
+    VERIFY_IS_LESS_THAN(tries, 5, L"Make sure we set up the new console in time");
 
     return true;
 }