A bunch of test fixes

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.
 

src/cascadia/LocalTests_SettingsModel/pch.h

@@ -34,6 +34,7 @@ Author(s):
 #include <WexTestClass.h>
 #include <json.h>
 #include "consoletaeftemplates.hpp"
+#include "winrtTaefTemplates.hpp"
 
 #include <winrt/Windows.ApplicationModel.Resources.Core.h>
 #include "winrt/Windows.UI.Xaml.Markup.h"

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<winrt::TerminalApp::implementation::CommandPalette>(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.
     }
 }

src/cascadia/LocalTests_TerminalApp/pch.h

@@ -34,6 +34,7 @@ Author(s):
 #include <WexTestClass.h>
 #include <json.h>
 #include "consoletaeftemplates.hpp"
+#include "winrtTaefTemplates.hpp"
 
 #include <winrt/Windows.ApplicationModel.Resources.Core.h>
 #include "winrt/Windows.UI.Xaml.Markup.h"

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()

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:
@@ -1373,8 +1376,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);
@@ -3182,8 +3185,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();
+            }
         }
     }
 
@@ -3195,8 +3205,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();
+            }
         }
     }
 

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()
         }
     }
 

src/cascadia/UnitTests_TerminalCore/pch.h

@@ -34,6 +34,7 @@ Author(s):
 
 #include <WexTestClass.h>
 #include "consoletaeftemplates.hpp"
+#include "winrtTaefTemplates.hpp"
 
 #include <winrt/Windows.system.h>
 #include <winrt/Windows.Foundation.h>

src/cascadia/ut_app/precomp.h

@@ -45,6 +45,8 @@ Author(s):
 #include <winrt/Windows.UI.Core.h>
 #include <winrt/Windows.UI.Text.h>
 
+#include "winrtTaefTemplates.hpp"
+
 // Manually include til after we include Windows.Foundation to give it winrt superpowers
 #include "til.h"
 

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<COORD> 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<COORD>. 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<>

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<COORD> 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<COORD>. 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<winrt::hstring>
+    {
+    public:
+        static WEX::Common::NoThrowString ToString(const winrt::hstring& hstr)
+        {
+            return WEX::Common::NoThrowString().Format(L"%s", hstr.c_str());
+        }
+    };
+}