diff --git a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp index 5aea35985..11d406428 100644 --- a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp @@ -275,14 +275,6 @@ namespace SettingsModelLocalTests void CommandTests::TestAutogeneratedName() { - // Tests run in Helix can't report Skipped until GH#7286 is resolved. - // Set ignore flag to make Helix run completely overlook it. - BEGIN_TEST_METHOD_PROPERTIES() - TEST_METHOD_PROPERTY(L"Ignore", L"True") - END_TEST_METHOD_PROPERTIES() - - // This test to be corrected as a part of GH#7281 - // This test ensures that we'll correctly create commands for actions // that don't have given names, pursuant to the spec in GH#6532. diff --git a/src/cascadia/LocalTests_SettingsModel/pch.h b/src/cascadia/LocalTests_SettingsModel/pch.h index 9a06e029c..672512676 100644 --- a/src/cascadia/LocalTests_SettingsModel/pch.h +++ b/src/cascadia/LocalTests_SettingsModel/pch.h @@ -34,6 +34,7 @@ Author(s): #include #include #include "consoletaeftemplates.hpp" +#include "winrtTaefTemplates.hpp" #include #include "winrt/Windows.UI.Xaml.Markup.h" diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index 2479f1628..4d11366db 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -868,13 +868,33 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(4u, page->_mruTabs.Size()); Log::Comment(L"give alphabetical names to all switch tab actions"); - RunOnUIThread([&page]() { + TestOnUIThread([&page]() { page->_GetTerminalTabImpl(page->_tabs.GetAt(0))->Title(L"a"); + }); + TestOnUIThread([&page]() { page->_GetTerminalTabImpl(page->_tabs.GetAt(1))->Title(L"b"); + }); + TestOnUIThread([&page]() { page->_GetTerminalTabImpl(page->_tabs.GetAt(2))->Title(L"c"); + }); + TestOnUIThread([&page]() { page->_GetTerminalTabImpl(page->_tabs.GetAt(3))->Title(L"d"); }); + TestOnUIThread([&page]() { + Log::Comment(L"Sanity check the titles of our tabs are what we set them to."); + + VERIFY_ARE_EQUAL(L"a", page->_tabs.GetAt(0).Title()); + VERIFY_ARE_EQUAL(L"b", page->_tabs.GetAt(1).Title()); + VERIFY_ARE_EQUAL(L"c", page->_tabs.GetAt(2).Title()); + VERIFY_ARE_EQUAL(L"d", page->_tabs.GetAt(3).Title()); + + VERIFY_ARE_EQUAL(L"d", page->_mruTabs.GetAt(0).Title()); + VERIFY_ARE_EQUAL(L"c", page->_mruTabs.GetAt(1).Title()); + VERIFY_ARE_EQUAL(L"b", page->_mruTabs.GetAt(2).Title()); + VERIFY_ARE_EQUAL(L"a", page->_mruTabs.GetAt(3).Title()); + }); + Log::Comment(L"Change the tab switch order to MRU switching"); TestOnUIThread([&page]() { page->_settings.GlobalSettings().TabSwitcherMode(TabSwitcherMode::MostRecentlyUsed); @@ -897,18 +917,30 @@ namespace TerminalAppLocalTests Log::Comment(L"Switch to the next MRU tab, which is the third tab"); RunOnUIThread([&page]() { page->_SelectNextTab(true); + // In the course of a single tick, the Command Palette will: + // * open + // * select the proper tab from the mru's list + // * raise an event for _filteredActionsView().SelectionChanged to + // immediately preview the new tab + // * raise a _SwitchToTabRequestedHandlers event + // * then dismiss itself, because we can't fake holing down an + // anchor key in the tests + }); + + TestOnUIThread([&page]() { + VERIFY_ARE_EQUAL(L"c", page->_mruTabs.GetAt(0).Title()); + VERIFY_ARE_EQUAL(L"d", page->_mruTabs.GetAt(1).Title()); + VERIFY_ARE_EQUAL(L"b", page->_mruTabs.GetAt(2).Title()); + VERIFY_ARE_EQUAL(L"a", page->_mruTabs.GetAt(3).Title()); }); const auto palette = winrt::get_self(page->CommandPalette()); - VERIFY_ARE_EQUAL(1u, palette->_switcherStartIdx, L"Verify the index is 1 as we went right"); VERIFY_ARE_EQUAL(winrt::TerminalApp::implementation::CommandPaletteMode::TabSwitchMode, palette->_currentMode, L"Verify we are in the tab switcher mode"); - - Log::Comment(L"Verify command palette preserves MRU order of tabs"); - VERIFY_ARE_EQUAL(4u, palette->_filteredActions.Size()); - VERIFY_ARE_EQUAL(L"d", palette->_filteredActions.GetAt(0).Item().Name()); - VERIFY_ARE_EQUAL(L"c", palette->_filteredActions.GetAt(1).Item().Name()); - VERIFY_ARE_EQUAL(L"b", palette->_filteredActions.GetAt(2).Item().Name()); - VERIFY_ARE_EQUAL(L"a", palette->_filteredActions.GetAt(3).Item().Name()); + // At this point, the contents of the command palette's _mruTabs list is + // still the _old_ ordering (d, c, b, a). The ordering is only updated + // in TerminalPage::_SelectNextTab, but as we saw before, the palette + // will also dismiss itself immediately when that's called. So we can't + // really inspect the contents of the list in this test, unfortunately. } } diff --git a/src/cascadia/LocalTests_TerminalApp/pch.h b/src/cascadia/LocalTests_TerminalApp/pch.h index eaf9c0448..95217495a 100644 --- a/src/cascadia/LocalTests_TerminalApp/pch.h +++ b/src/cascadia/LocalTests_TerminalApp/pch.h @@ -34,6 +34,7 @@ Author(s): #include #include #include "consoletaeftemplates.hpp" +#include "winrtTaefTemplates.hpp" #include #include "winrt/Windows.UI.Xaml.Markup.h" diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 5880e9d56..6f2ec734b 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -172,6 +172,9 @@ namespace winrt::TerminalApp::implementation // Method Description: // - Returns the settings currently in use by the entire Terminal application. + // - IMPORTANT! This can throw! Make sure to try/catch this, so that the + // LocalTests don't crash (because their Application::Current() won't be a + // AppLogic) // Throws: // - HR E_INVALIDARG if the app isn't up and running. const CascadiaSettings AppLogic::CurrentAppSettings() diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 595a99876..b96a0e7cc 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -135,8 +135,7 @@ namespace winrt::TerminalApp::implementation } CATCH_LOG(); - _tabRow.PointerMoved({ this, &TerminalPage::_RestorePointerCursorHandler }); - + _tabRow.PointerMoved({ get_weak(), &TerminalPage::_RestorePointerCursorHandler }); _tabView.CanReorderTabs(!isElevated); _tabView.CanDragTabs(!isElevated); @@ -261,7 +260,11 @@ namespace winrt::TerminalApp::implementation // Store cursor, so we can restore it, e.g., after mouse vanishing // (we'll need to adapt this logic once we make cursor context aware) - _defaultPointerCursor = CoreWindow::GetForCurrentThread().PointerCursor(); + try + { + _defaultPointerCursor = CoreWindow::GetForCurrentThread().PointerCursor(); + } + CATCH_LOG(); } // Method Description: @@ -1387,8 +1390,8 @@ namespace winrt::TerminalApp::implementation // Add an event handler for when the terminal wants to set a progress indicator on the taskbar term.SetTaskbarProgress({ this, &TerminalPage::_SetTaskbarProgressHandler }); - term.HidePointerCursor({ this, &TerminalPage::_HidePointerCursorHandler }); - term.RestorePointerCursor({ this, &TerminalPage::_RestorePointerCursorHandler }); + term.HidePointerCursor({ get_weak(), &TerminalPage::_HidePointerCursorHandler }); + term.RestorePointerCursor({ get_weak(), &TerminalPage::_RestorePointerCursorHandler }); // Bind Tab events to the TermControl and the Tab's Pane hostingTab.Initialize(term); @@ -3196,8 +3199,15 @@ namespace winrt::TerminalApp::implementation { if (_shouldMouseVanish && !_isMouseHidden) { - CoreWindow::GetForCurrentThread().PointerCursor(nullptr); - _isMouseHidden = true; + if (auto window{ CoreWindow::GetForCurrentThread() }) + { + try + { + window.PointerCursor(nullptr); + _isMouseHidden = true; + } + CATCH_LOG(); + } } } @@ -3209,8 +3219,15 @@ namespace winrt::TerminalApp::implementation { if (_isMouseHidden) { - CoreWindow::GetForCurrentThread().PointerCursor(_defaultPointerCursor); - _isMouseHidden = false; + if (auto window{ CoreWindow::GetForCurrentThread() }) + { + try + { + window.PointerCursor(_defaultPointerCursor); + _isMouseHidden = false; + } + CATCH_LOG(); + } } } diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 5043b90c8..e290cee30 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -104,15 +104,21 @@ namespace winrt::TerminalApp::implementation if (auto tab{ weakThis.get() }) { - const auto settings{ winrt::TerminalApp::implementation::AppLogic::CurrentAppSettings() }; - if (settings.GlobalSettings().TabWidthMode() == winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode::SizeToContent) + try { - tab->_headerControl.RenamerMaxWidth(HeaderRenameBoxWidthTitleLength); - } - else - { - tab->_headerControl.RenamerMaxWidth(HeaderRenameBoxWidthDefault); + // Make sure to try/catch this, because the LocalTests won't be + // able to use this helper. + const auto settings{ winrt::TerminalApp::implementation::AppLogic::CurrentAppSettings() }; + if (settings.GlobalSettings().TabWidthMode() == winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode::SizeToContent) + { + tab->_headerControl.RenamerMaxWidth(HeaderRenameBoxWidthTitleLength); + } + else + { + tab->_headerControl.RenamerMaxWidth(HeaderRenameBoxWidthDefault); + } } + CATCH_LOG() } } diff --git a/src/cascadia/UnitTests_TerminalCore/pch.h b/src/cascadia/UnitTests_TerminalCore/pch.h index c33ba50c2..799e3c89b 100644 --- a/src/cascadia/UnitTests_TerminalCore/pch.h +++ b/src/cascadia/UnitTests_TerminalCore/pch.h @@ -34,6 +34,7 @@ Author(s): #include #include "consoletaeftemplates.hpp" +#include "winrtTaefTemplates.hpp" #include #include diff --git a/src/cascadia/ut_app/precomp.h b/src/cascadia/ut_app/precomp.h index 51ff1f447..bb5f4eaa2 100644 --- a/src/cascadia/ut_app/precomp.h +++ b/src/cascadia/ut_app/precomp.h @@ -45,6 +45,8 @@ Author(s): #include #include +#include "winrtTaefTemplates.hpp" + // Manually include til after we include Windows.Foundation to give it winrt superpowers #include "til.h" diff --git a/src/inc/consoletaeftemplates.hpp b/src/inc/consoletaeftemplates.hpp index 592b9d09b..fcd0fa6c6 100644 --- a/src/inc/consoletaeftemplates.hpp +++ b/src/inc/consoletaeftemplates.hpp @@ -22,6 +22,25 @@ Revision History: type identifer; \ VERIFY_SUCCEEDED(TestData::TryGetValue(L#identifer, identifer), description); +// Thinking of adding a new VerifyOutputTraits for a new type? MAKE SURE that +// you include this header (or at least the relevant definition) before _every_ +// Verify for that type. +// +// From a thread on the matter in 2018: +// > my best guess is that one of your cpp files uses a COORD in a Verify macro +// > without including consoletaeftemplates.hpp. This caused the +// > VerifyOutputTraits symbol to be used with the default +// > implementation. In other of your cpp files, you did include +// > consoletaeftemplates.hpp properly and they would have compiled the actual +// > specialization from consoletaeftemplates.hpp into the corresponding obj +// > file for that cpp file. When the test DLL was linked, the linker picks one +// > of the multiple definitions available from the different obj files for +// > VerifyOutputTraits. The linker happened to pick the one from the cpp +// > file where consoletaeftemplates.hpp was not included properly. I’ve +// > encountered a similar situation before and it was baffling because the +// > compiled code was obviously doing different behavior than what the source +// > code said. This can happen when you violate the one-definition rule. + namespace WEX::TestExecution { template<> diff --git a/src/inc/winrtTaefTemplates.hpp b/src/inc/winrtTaefTemplates.hpp new file mode 100644 index 000000000..e8b196b2b --- /dev/null +++ b/src/inc/winrtTaefTemplates.hpp @@ -0,0 +1,50 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- winrtTaefTemplates.hpp + +Abstract: +- This module contains common TAEF templates for winrt-isms that don't otherwise + have them. This is very similar to consoleTaefTemplates, but this one presumes + that the winrt headers have already been included. + +Author: +- Mike Griese 2021 + +--*/ + +#pragma once + +// Thinking of adding a new VerifyOutputTraits for a new type? MAKE SURE that +// you include this header (or at least the relevant definition) before _every_ +// Verify for that type. +// +// From a thread on the matter in 2018: +// > my best guess is that one of your cpp files uses a COORD in a Verify macro +// > without including consoletaeftemplates.hpp. This caused the +// > VerifyOutputTraits symbol to be used with the default +// > implementation. In other of your cpp files, you did include +// > consoletaeftemplates.hpp properly and they would have compiled the actual +// > specialization from consoletaeftemplates.hpp into the corresponding obj +// > file for that cpp file. When the test DLL was linked, the linker picks one +// > of the multiple definitions available from the different obj files for +// > VerifyOutputTraits. The linker happened to pick the one from the cpp +// > file where consoletaeftemplates.hpp was not included properly. I’ve +// > encountered a similar situation before and it was baffling because the +// > compiled code was obviously doing different behavior than what the source +// > code said. This can happen when you violate the one-definition rule. + +namespace WEX::TestExecution +{ + template<> + class VerifyOutputTraits + { + public: + static WEX::Common::NoThrowString ToString(const winrt::hstring& hstr) + { + return WEX::Common::NoThrowString().Format(L"%s", hstr.c_str()); + } + }; +}