Skip to content

Commit

Permalink
Move tests to invoke te.exe directly instead of using VSTest runner (
Browse files Browse the repository at this point in the history
…#4490)

Moves the tests from using the `vstest.console.exe` route to just using `te.exe`.

PROs:
- `te.exe` is significantly faster for running tests because the TAEF/VSTest adapter isn't great.
- Running through `te.exe` is closer to what our developers are doing on their dev boxes
- `te.exe` is how they run in the Windows gates.
- `te.exe` doesn't seem to have the sporadic `0x6` error code thrown during the tests where somehow the console handles get lost
- `te.exe` doesn't seem to repro the other intermittent issues that we have been having that are inscrutable. 
- Fewer processes in the tree (te is running anyway under `vstest.console.exe`, just indirected a lot
- The log outputs scroll live with all our logging messages instead of suppressing everything until there's a failure
- The log output is actually in the order things are happening versus vstest.

CONs:
- No more code coverage.
- No more test records in the ADO build/test panel.
- Tests really won't work inside Visual Studio at all.
- The log files are really big now
- Testing is not a test task anymore, just another script.

Refuting each CON:
- We didn't read the code coverage numbers
- We didn't look at the ADO test panel results or build-over-build velocities
- Tests didn't really work inside Visual Studio anyway unless you did the right incantations under the full moon.
- We could tone down the logging if we wanted at either the te.exe execution time (with a switch) or by declaring properties in the tests/classes/modules that are very verbose to not log unless it fails.
- I don't think anyone cares how they get run as long as they do.
  • Loading branch information
miniksa authored Feb 10, 2020
1 parent 46b70d8 commit 86706d7
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 89 deletions.
46 changes: 36 additions & 10 deletions OpenConsole.sln
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,6 @@ EndProject
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Propsheet.DLL", "src\propsheet\propsheet.vcxproj", "{5D23E8E1-3C64-4CC1-A8F7-6861677F7239}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "_Build Common", "_Build Common", "{04170EEF-983A-4195-BFEF-2321E5E38A1E}"
ProjectSection(SolutionItems) = preProject
src\common.build.post.props = src\common.build.post.props
src\common.build.pre.props = src\common.build.pre.props
src\common.build.tests.props = src\common.build.tests.props
common.openconsole.props = common.openconsole.props
src\cppwinrt.build.post.props = src\cppwinrt.build.post.props
src\cppwinrt.build.pre.props = src\cppwinrt.build.pre.props
src\wap-common.build.post.props = src\wap-common.build.post.props
src\wap-common.build.pre.props = src\wap-common.build.pre.props
EndProjectSection
EndProject
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Server", "src\server\lib\server.vcxproj", "{18D09A24-8240-42D6-8CB6-236EEE820262}"
EndProject
Expand Down Expand Up @@ -271,6 +261,39 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "til.unit.tests", "src\til\u
EndProject
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "U8U16Test", "src\tools\U8U16Test\U8U16Test.vcxproj", "{A602A555-BAAC-46E1-A91D-3DAB0475C5A1}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Common Props", "Common Props", "{53DD5520-E64C-4C06-B472-7CE62CA539C9}"
ProjectSection(SolutionItems) = preProject
src\common.build.post.props = src\common.build.post.props
src\common.build.pre.props = src\common.build.pre.props
src\common.build.tests.props = src\common.build.tests.props
common.openconsole.props = common.openconsole.props
src\cppwinrt.build.post.props = src\cppwinrt.build.post.props
src\cppwinrt.build.pre.props = src\cppwinrt.build.pre.props
src\wap-common.build.post.props = src\wap-common.build.post.props
src\wap-common.build.pre.props = src\wap-common.build.pre.props
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "YAML", "YAML", "{6B5A44ED-918D-4747-BFB1-2472A1FCA173}"
ProjectSection(SolutionItems) = preProject
build\pipelines\templates\build-console-audit-job.yml = build\pipelines\templates\build-console-audit-job.yml
build\pipelines\templates\build-console-ci.yml = build\pipelines\templates\build-console-ci.yml
build\pipelines\templates\build-console-int.yml = build\pipelines\templates\build-console-int.yml
build\pipelines\templates\build-console-steps.yml = build\pipelines\templates\build-console-steps.yml
build\pipelines\templates\check-formatting.yml = build\pipelines\templates\check-formatting.yml
build\pipelines\ci.yml = build\pipelines\ci.yml
build\pipelines\templates\release-sign-and-bundle.yml = build\pipelines\templates\release-sign-and-bundle.yml
build\pipelines\release.yml = build\pipelines\release.yml
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Scripts", "Scripts", "{D3EF7B96-CD5E-47C9-B9A9-136259563033}"
ProjectSection(SolutionItems) = preProject
build\scripts\Create-AppxBundle.ps1 = build\scripts\Create-AppxBundle.ps1
build\scripts\Index-Pdbs.ps1 = build\scripts\Index-Pdbs.ps1
build\scripts\Invoke-FormattingCheck.ps1 = build\scripts\Invoke-FormattingCheck.ps1
build\scripts\Run-Tests.ps1 = build\scripts\Run-Tests.ps1
build\scripts\Test-WindowsTerminalPackage.ps1 = build\scripts\Test-WindowsTerminalPackage.ps1
EndProjectSection
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
AuditMode|Any CPU = AuditMode|Any CPU
Expand Down Expand Up @@ -1467,6 +1490,9 @@ Global
{BDB237B6-1D1D-400F-84CC-40A58FA59C8E} = {59840756-302F-44DF-AA47-441A9D673202}
{767268EE-174A-46FE-96F0-EEE698A1BBC9} = {89CDCC5C-9F53-4054-97A4-639D99F169CD}
{A602A555-BAAC-46E1-A91D-3DAB0475C5A1} = {A10C4720-DCA4-4640-9749-67F4314F527C}
{53DD5520-E64C-4C06-B472-7CE62CA539C9} = {04170EEF-983A-4195-BFEF-2321E5E38A1E}
{6B5A44ED-918D-4747-BFB1-2472A1FCA173} = {04170EEF-983A-4195-BFEF-2321E5E38A1E}
{D3EF7B96-CD5E-47C9-B9A9-136259563033} = {04170EEF-983A-4195-BFEF-2321E5E38A1E}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {3140B1B7-C8EE-43D1-A772-D82A7061A271}
Expand Down
37 changes: 17 additions & 20 deletions build/pipelines/templates/build-console-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,32 +52,29 @@ steps:
arguments: -SearchDir '$(Build.SourcesDirectory)' -SourceRoot '$(Build.SourcesDirectory)' -recursive -Verbose -CommitId $(Build.SourceVersion)
errorActionPreference: silentlyContinue

- task: VSTest@2
- task: PowerShell@2
displayName: 'Rationalize build platform'
inputs:
targetType: inline
script: |
$Arch = "$(BuildPlatform)"
If ($Arch -Eq "x86") { $Arch = "Win32" }
Write-Host "##vso[task.setvariable variable=RationalizedBuildPlatform]${Arch}"
- task: PowerShell@2
displayName: 'Run Unit Tests'
inputs:
testAssemblyVer2: |
$(BUILD.SOURCESDIRECTORY)\**\*unit.test*.dll
!**\obj\**
runSettingsFile: '$(BUILD.SOURCESDIRECTORY)\src\unit.tests.$(BuildPlatform).runsettings'
codeCoverageEnabled: true
runInParallel: False
testRunTitle: 'Console Unit Tests'
platform: '$(BuildPlatform)'
configuration: '$(BuildConfiguration)'
targetType: filePath
filePath: build\scripts\Run-Tests.ps1
arguments: -MatchPattern '*unit.test*.dll' -Platform '$(RationalizedBuildPlatform)' -Configuration '$(BuildConfiguration)'
condition: and(succeeded(), or(eq(variables['BuildPlatform'], 'x64'), eq(variables['BuildPlatform'], 'x86')))

- task: VSTest@2
- task: PowerShell@2
displayName: 'Run Feature Tests (x64 only)'
inputs:
testAssemblyVer2: |
$(BUILD.SOURCESDIRECTORY)\**\*feature.test*.dll
!**\obj\**
runSettingsFile: '$(BUILD.SOURCESDIRECTORY)\src\unit.tests.$(BuildPlatform).runsettings'
codeCoverageEnabled: true
runInParallel: False
testRunTitle: 'Console Feature Tests'
platform: '$(BuildPlatform)'
configuration: '$(BuildConfiguration)'
targetType: filePath
filePath: build\scripts\Run-Tests.ps1
arguments: -MatchPattern '*feature.test*.dll' -Platform '$(RationalizedBuildPlatform)' -Configuration '$(BuildConfiguration)'
condition: and(succeeded(), eq(variables['BuildPlatform'], 'x64'))

- task: CopyFiles@2
Expand Down
14 changes: 14 additions & 0 deletions build/scripts/Run-Tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[CmdLetBinding()]
Param(
[Parameter(Mandatory=$true, Position=0)][string]$MatchPattern,
[Parameter(Mandatory=$true, Position=1)][string]$Platform,
[Parameter(Mandatory=$true, Position=2)][string]$Configuration
)

$testdlls = Get-ChildItem -Path ".\bin\$Platform\$Configuration" -Recurse -Filter $MatchPattern

&".\bin\$Platform\$Configuration\te.exe" $testdlls.FullName

if ($lastexitcode -Ne 0) { Exit $lastexitcode }

Exit 0
114 changes: 59 additions & 55 deletions src/host/ft_host/API_InputTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ void InputTests::TestReadConsolePasswordScenario()
VERIFY_ARE_EQUAL(wcslen(pwszExpected), len);
}

void TestMouseWheelReadConsoleInputHelper(const UINT msg, const DWORD dwEventFlagsExpected, const DWORD dwConsoleMode)
void TestMouseWheelReadConsoleInputHelper(const UINT /*msg*/, const DWORD /*dwEventFlagsExpected*/, const DWORD /*dwConsoleMode*/)
{
if (!OneCoreDelay::IsIsWindowPresent())
{
Expand All @@ -409,60 +409,64 @@ void TestMouseWheelReadConsoleInputHelper(const UINT msg, const DWORD dwEventFla
return;
}

HWND const hwnd = GetConsoleWindow();
VERIFY_IS_TRUE(!!IsWindow(hwnd), L"Get console window handle to inject wheel messages.");

HANDLE const hConsoleInput = GetStdInputHandle();
VERIFY_WIN32_BOOL_SUCCEEDED(SetConsoleMode(hConsoleInput, dwConsoleMode), L"Apply the requested console mode");

// We don't generate mouse console event in QuickEditMode or if MouseInput is not enabled
DWORD dwExpectedEvents = 1;
if (dwConsoleMode & ENABLE_QUICK_EDIT_MODE || !(dwConsoleMode & ENABLE_MOUSE_INPUT))
{
Log::Comment(L"QuickEditMode is set or MouseInput is not set, not expecting events");
dwExpectedEvents = 0;
}

VERIFY_WIN32_BOOL_SUCCEEDED(FlushConsoleInputBuffer(hConsoleInput), L"Flush input queue to make sure no one else is in the way.");

// WM_MOUSEWHEEL params
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms645617(v=vs.85).aspx

// WPARAM is HIWORD the wheel delta and LOWORD the keystate (keys pressed with it)
// We want no keys pressed in the loword (0) and we want one tick of the wheel in the high word.
WPARAM wParam = 0;
short sKeyState = 0;
short sWheelDelta = -WHEEL_DELTA; // scroll down is negative, up is positive.
// we only use the lower 32-bits (in case of 64-bit system)
wParam = ((sWheelDelta << 16) | sKeyState) & 0xFFFFFFFF;

// LPARAM is positioning information. We don't care so we'll leave it 0x0
LPARAM lParam = 0;

Log::Comment(L"Send scroll down message into console window queue.");
SendMessageW(hwnd, msg, wParam, lParam);

Sleep(250); // give message time to sink in

DWORD dwAvailable = 0;
VERIFY_WIN32_BOOL_SUCCEEDED(GetNumberOfConsoleInputEvents(hConsoleInput, &dwAvailable), L"Retrieve number of events in queue.");
VERIFY_ARE_EQUAL(dwExpectedEvents, dwAvailable, NoThrowString().Format(L"We expected %i event from our scroll message.", dwExpectedEvents));

INPUT_RECORD ir;
DWORD dwRead = 0;
if (dwExpectedEvents == 1)
{
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleInputW(hConsoleInput, &ir, 1, &dwRead), L"Read the event out.");
VERIFY_ARE_EQUAL(1u, dwRead);

Log::Comment(L"Verify the event is what we expected. We only verify the fields relevant to this test.");
VERIFY_ARE_EQUAL(MOUSE_EVENT, ir.EventType);
// hard cast OK. only using lower 32-bits (see above)
VERIFY_ARE_EQUAL((DWORD)wParam, ir.Event.MouseEvent.dwButtonState);
// Don't care about ctrl key state. Can be messed with by caps lock/numlock state. Not checking this.
VERIFY_ARE_EQUAL(dwEventFlagsExpected, ir.Event.MouseEvent.dwEventFlags);
// Don't care about mouse position for ensuring scroll message went through.
}
Log::Comment(L"This test is flaky. Fix me in GH#4494");
Log::Result(WEX::Logging::TestResults::Skipped);
return;

//HWND const hwnd = GetConsoleWindow();
//VERIFY_IS_TRUE(!!IsWindow(hwnd), L"Get console window handle to inject wheel messages.");

//HANDLE const hConsoleInput = GetStdInputHandle();
//VERIFY_WIN32_BOOL_SUCCEEDED(SetConsoleMode(hConsoleInput, dwConsoleMode), L"Apply the requested console mode");

//// We don't generate mouse console event in QuickEditMode or if MouseInput is not enabled
//DWORD dwExpectedEvents = 1;
//if (dwConsoleMode & ENABLE_QUICK_EDIT_MODE || !(dwConsoleMode & ENABLE_MOUSE_INPUT))
//{
// Log::Comment(L"QuickEditMode is set or MouseInput is not set, not expecting events");
// dwExpectedEvents = 0;
//}

//VERIFY_WIN32_BOOL_SUCCEEDED(FlushConsoleInputBuffer(hConsoleInput), L"Flush input queue to make sure no one else is in the way.");

//// WM_MOUSEWHEEL params
//// https://msdn.microsoft.com/en-us/library/windows/desktop/ms645617(v=vs.85).aspx

//// WPARAM is HIWORD the wheel delta and LOWORD the keystate (keys pressed with it)
//// We want no keys pressed in the loword (0) and we want one tick of the wheel in the high word.
//WPARAM wParam = 0;
//short sKeyState = 0;
//short sWheelDelta = -WHEEL_DELTA; // scroll down is negative, up is positive.
//// we only use the lower 32-bits (in case of 64-bit system)
//wParam = ((sWheelDelta << 16) | sKeyState) & 0xFFFFFFFF;

//// LPARAM is positioning information. We don't care so we'll leave it 0x0
//LPARAM lParam = 0;

//Log::Comment(L"Send scroll down message into console window queue.");
//SendMessageW(hwnd, msg, wParam, lParam);

//Sleep(250); // give message time to sink in

//DWORD dwAvailable = 0;
//VERIFY_WIN32_BOOL_SUCCEEDED(GetNumberOfConsoleInputEvents(hConsoleInput, &dwAvailable), L"Retrieve number of events in queue.");
//VERIFY_ARE_EQUAL(dwExpectedEvents, dwAvailable, NoThrowString().Format(L"We expected %i event from our scroll message.", dwExpectedEvents));

//INPUT_RECORD ir;
//DWORD dwRead = 0;
//if (dwExpectedEvents == 1)
//{
// VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleInputW(hConsoleInput, &ir, 1, &dwRead), L"Read the event out.");
// VERIFY_ARE_EQUAL(1u, dwRead);

// Log::Comment(L"Verify the event is what we expected. We only verify the fields relevant to this test.");
// VERIFY_ARE_EQUAL(MOUSE_EVENT, ir.EventType);
// // hard cast OK. only using lower 32-bits (see above)
// VERIFY_ARE_EQUAL((DWORD)wParam, ir.Event.MouseEvent.dwButtonState);
// // Don't care about ctrl key state. Can be messed with by caps lock/numlock state. Not checking this.
// VERIFY_ARE_EQUAL(dwEventFlagsExpected, ir.Event.MouseEvent.dwEventFlags);
// // Don't care about mouse position for ensuring scroll message went through.
//}
}

void InputTests::TestMouseWheelReadConsoleMouseInput()
Expand Down
4 changes: 3 additions & 1 deletion src/host/ut_host/ConptyOutputTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ using namespace Microsoft::Console::Types;

class ConptyOutputTests
{
TEST_CLASS(ConptyOutputTests);
BEGIN_TEST_CLASS(ConptyOutputTests)
TEST_CLASS_PROPERTY(L"IsolationLevel", L"Class")
END_TEST_CLASS()

TEST_CLASS_SETUP(ClassSetup)
{
Expand Down
8 changes: 5 additions & 3 deletions src/host/ut_host/VtIoTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ using namespace std;

class Microsoft::Console::VirtualTerminal::VtIoTests
{
TEST_CLASS(VtIoTests);
BEGIN_TEST_CLASS(VtIoTests)
TEST_CLASS_PROPERTY(L"IsolationLevel", L"Class")
END_TEST_CLASS()

// General Tests:
TEST_METHOD(NoOpStartTest);
Expand Down Expand Up @@ -360,7 +362,7 @@ void VtIoTests::RendererDtorAndThread()
// EnablePainting gets called, and if that happens, then the thread will
// never get destructed. This will only ever happen in the vstest test runner,
// which is what CI uses.
Sleep(500);
/*Sleep(500);*/

pThread->EnablePainting();
pRenderer->TriggerTeardown();
Expand All @@ -387,7 +389,7 @@ void VtIoTests::RendererDtorAndThreadAndDx()
// EnablePainting gets called, and if that happens, then the thread will
// never get destructed. This will only ever happen in the vstest test runner,
// which is what CI uses.
Sleep(500);
/*Sleep(500);*/

pThread->EnablePainting();
pRenderer->TriggerTeardown();
Expand Down
6 changes: 6 additions & 0 deletions src/renderer/base/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Renderer::Renderer(IRenderData* pData,
Renderer::~Renderer()
{
_destructing = true;
_pThread.reset();
}

// Routine Description:
Expand All @@ -67,6 +68,11 @@ Renderer::~Renderer()
auto tries = maxRetriesForRenderEngine;
while (tries > 0)
{
if (_destructing)
{
return S_FALSE;
}

const auto hr = _PaintFrameForEngine(pEngine);
if (E_PENDING == hr)
{
Expand Down

0 comments on commit 86706d7

Please sign in to comment.