Skip to content

Commit

Permalink
Fix the feature tests with a retry (microsoft#8534)
Browse files Browse the repository at this point in the history
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 microsoft#8495
  • Loading branch information
zadjii-msft authored and mpela81 committed Jan 28, 2021
1 parent 025acfd commit f42bf6f
Showing 1 changed file with 39 additions and 11 deletions.
50 changes: 39 additions & 11 deletions src/host/ft_host/InitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit f42bf6f

Please sign in to comment.