Repository: microsoft/terminal (language: cpp)

Issue:
SerializationTests::Actions started failing recently
### Windows Terminal version

Commit 848314ef17419430153561016d1fabe912338844

### Windows build number

10.0.19044.1706

### Other Software

_No response_

### Steps to reproduce

1. Build a recent commit of OpenConsole (I think anything after 799b5d4add799ac2fbf5c855b774cd7dcb957697 should do).
2. Run the unit tests.

### Expected Behavior

All the tests should pass.

### Actual Behavior

There is a failure in `SettingsModelLocalTests::SerializationTests::Actions` when attempting to serialize the pair of `findMatch` actions. It seems that the commands are being reordered.

<details>
  <summary>Click to see error</summary>
  
```
Error: Verify: AreEqual(toString(json), toString(result)) - Values ([
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "next"
                },
                "keys" : "ctrl+shift+s"
        },
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "prev"
                },
                "keys" : "ctrl+shift+r"
        }
], [
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "prev"
                },
                "keys" : "ctrl+shift+r"
        },
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "next"
                },
                "keys" : "ctrl+shift+s"
        }
])
```
</details>

I'm not sure if this is actually an indication of a bug - is the order of the commands expected to be retained? But at the very least I'd expect the test to be corrected if this is just an altered hash map order or something of that sort.

If it helps, I think the first time the test stopped working was after commit 799b5d4add799ac2fbf5c855b774cd7dcb957697.
SerializationTests::Actions started failing recently
### Windows Terminal version

Commit 848314ef17419430153561016d1fabe912338844

### Windows build number

10.0.19044.1706

### Other Software

_No response_

### Steps to reproduce

1. Build a recent commit of OpenConsole (I think anything after 799b5d4add799ac2fbf5c855b774cd7dcb957697 should do).
2. Run the unit tests.

### Expected Behavior

All the tests should pass.

### Actual Behavior

There is a failure in `SettingsModelLocalTests::SerializationTests::Actions` when attempting to serialize the pair of `findMatch` actions. It seems that the commands are being reordered.

<details>
  <summary>Click to see error</summary>
  
```
Error: Verify: AreEqual(toString(json), toString(result)) - Values ([
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "next"
                },
                "keys" : "ctrl+shift+s"
        },
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "prev"
                },
                "keys" : "ctrl+shift+r"
        }
], [
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "prev"
                },
                "keys" : "ctrl+shift+r"
        },
        {
                "command" :
                {
                        "action" : "findMatch",
                        "direction" : "next"
                },
                "keys" : "ctrl+shift+s"
        }
])
```
</details>

I'm not sure if this is actually an indication of a bug - is the order of the commands expected to be retained? But at the very least I'd expect the test to be corrected if this is just an altered hash map order or something of that sort.

If it helps, I think the first time the test stopped working was after commit 799b5d4add799ac2fbf5c855b774cd7dcb957697.


Discussion / hints:
> If it helps, I think the first time the test stopped working was after commit https://github.com/microsoft/terminal/commit/799b5d4add799ac2fbf5c855b774cd7dcb957697.

That's w e i r d, Cause, that shouldn't really affect this at all 🙃

I don't think the order really matters at all tbh, string comparing the json was just a shortcut vs having to actually re-parse and structurally analyze for equality. Let's just swap the order around (if it's seemingly stable in the new order)
I've got a fix in, well... 5a40e8061 I think. I think I accidentally did a `git add --all` there, so it's a bit tangled with other prototypes. I'll submit the PR tomorrow.
Note to self: `dev/migrie/b/12788-did-it-work`. That's the branch where I tried to do the CI changes that _really_ did not work. 
> If it helps, I think the first time the test stopped working was after commit https://github.com/microsoft/terminal/commit/799b5d4add799ac2fbf5c855b774cd7dcb957697.

That's w e i r d, Cause, that shouldn't really affect this at all 🙃

I don't think the order really matters at all tbh, string comparing the json was just a shortcut vs having to actually re-parse and structurally analyze for equality. Let's just swap the order around (if it's seemingly stable in the new order)
I've got a fix in, well... 5a40e8061 I think. I think I accidentally did a `git add --all` there, so it's a bit tangled with other prototypes. I'll submit the PR tomorrow.
Note to self: `dev/migrie/b/12788-did-it-work`. That's the branch where I tried to do the CI changes that _really_ did not work. 

Fix patch:
diff --git a/src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp b/src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
index ff2aefdb6b8..f0a3ed3d02a 100644
--- a/src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
+++ b/src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
@@ -253,9 +253,14 @@ namespace SettingsModelLocalTests
                                                 { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+c" },
                                                 { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+d" }
                                             ])" };
+        // GH#13323 - these can be fragile. In the past, the order these get
+        // re-serialized as has been not entirely stable. We don't really care
+        // about the order they get re-serialized in, but the tests aren't
+        // clever enough to compare the structure, only the literal string
+        // itself. Feel free to change as needed.
         static constexpr std::string_view actionsString4B{ R"([
-                                                { "command": { "action": "findMatch", "direction": "next" }, "keys": "ctrl+shift+s" },
-                                                { "command": { "action": "findMatch", "direction": "prev" }, "keys": "ctrl+shift+r" }
+                                                { "command": { "action": "findMatch", "direction": "prev" }, "keys": "ctrl+shift+r" },
+                                                { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+d" }
                                             ])" };
 
         // command with name and icon and multiple key chords
@@ -455,7 +460,9 @@ namespace SettingsModelLocalTests
             ],
             "actions": [
                 { "command": { "action": "sendInput", "input": "VT Griese Mode" }, "keys": "ctrl+k" }
-            ]
+            ],
+            "theme": "system",
+            "themes": []
         })" };
 
         const auto settings{ winrt::make_self<implementation::CascadiaSettings>(settingsString) };
diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
index 4154786c303..a34183d80b0 100644
--- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
+++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
@@ -1156,6 +1156,20 @@ void CascadiaSettings::ExportFile(winrt::hstring path, winrt::hstring content)
 
 void CascadiaSettings::_validateThemeExists()
 {
+    if (_globals->Themes().Size() == 0)
+    {
+        // We didn't even load the default themes. This should only be possible
+        // if the defaults.json didn't include any themes, or if no
+        // defaults.json was loaded at all. The second case is especially common
+        // in tests (that don't bother with a defaults.json). No matter. Create
+        // a default theme under `system` and just stick it in there.
+
+        auto newTheme = winrt::make_self<Theme>();
+        newTheme->Name(L"system");
+        _globals->AddTheme(*newTheme);
+        _globals->Theme(L"system");
+    }
+
     if (!_globals->Themes().HasKey(_globals->Theme()))
     {
         _warnings.Append(SettingsLoadWarnings::UnknownTheme);


Test patch: