some new tests

src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp

@@ -157,17 +157,17 @@ namespace SettingsModelUnitTests
  void KeyBindingsTests::HashDeduplication()
  {
  const auto actionMap = winrt::make_self<implementation::ActionMap>();
- actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "id": "Test.SplitPane", "keys": ["ctrl+c"] } ])"), OriginTag::None);
- actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "id": "Test.SplitPane", "keys": ["ctrl+c"] } ])"), OriginTag::None);
+ actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "keys": ["ctrl+c"] } ])"), OriginTag::User);
+ actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "keys": ["ctrl+c"] } ])"), OriginTag::User);
  VERIFY_ARE_EQUAL(1u, actionMap->_ActionMap.size());
  }
 
  void KeyBindingsTests::HashContentArgs()
  {
- Log::Comment(L"These are two actions with different content args. They should have different hashes for their terminal args.");
+ Log::Comment(L"These are two actions with different content args. They should have different generated IDs for their terminal args.");
  const auto actionMap = winrt::make_self<implementation::ActionMap>();
- actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", } , "id": "Test.NewTabNoArgs", "keys": ["ctrl+c"] } ])"), OriginTag::None);
- actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", "index": 0 } , "id": "Test.NewTab0", "keys": ["ctrl+shift+c"] } ])"), OriginTag::None);
+ actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", } , "keys": ["ctrl+c"] } ])"), OriginTag::User);
+ actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", "index": 0 } , "keys": ["ctrl+shift+c"] } ])"), OriginTag::User);
  VERIFY_ARE_EQUAL(2u, actionMap->_ActionMap.size());
 
  KeyChord ctrlC{ VirtualKeyModifiers::Control, static_cast<int32_t>('C'), 0 };

src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp

@@ -47,6 +47,8 @@ namespace SettingsModelUnitTests
  TEST_METHOD(RoundtripGenerateActionID);
  TEST_METHOD(NoGeneratedIDsForIterableAndNestedCommands);
  TEST_METHOD(GeneratedActionIDsEqualForIdenticalCommands);
+ TEST_METHOD(RoundtripLegacyToModernActions);
+ TEST_METHOD(MultipleActionsAreCollapsed);
 
  private:
  // Method Description:
@@ -1074,4 +1076,123 @@ namespace SettingsModelUnitTests
 
  VERIFY_ARE_EQUAL(sendInputCmd1.ID(), sendInputCmd1.ID());
  }
+
+ void SerializationTests::RoundtripLegacyToModernActions()
+ {
+ static constexpr std::string_view oldSettingsJson{ R"(
+ {
+ "actions": [
+ {
+ "name": "foo",
+ "id": "Test.SendInput",
+ "command": { "action": "sendInput", "input": "just some input" },
+ "keys": "ctrl+shift+w"
+ },
+ {
+ "command": "unbound",
+ "keys": "ctrl+shift+x"
+ }
+ ]
+ })" };
+
+ // modern style:
+ // - no "unbound" actions, these are just keybindings that have no id
+ // - no keys in actions, these are keybindings with an id
+ static constexpr std::string_view newSettingsJson{ R"(
+ {
+ "actions": [
+ {
+ "name": "foo",
+ "command": { "action": "sendInput", "input": "just some input" },
+ "id": "Test.SendInput"
+ }
+ ],
+ "keybindings": [
+ {
+ "id": "Test.SendInput",
+ "keys": "ctrl+shift+w"
+ },
+ {
+ "id": null,
+ "keys": "ctrl+shift+x"
+ }
+ ]
+ })" };
+
+ implementation::SettingsLoader loader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
+ loader.MergeInboxIntoUserSettings();
+ loader.FinalizeLayering();
+ VERIFY_IS_TRUE(loader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
+ const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
+ const auto oldResult{ settings->ToJson() };
+
+ implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
+ newLoader.MergeInboxIntoUserSettings();
+ newLoader.FinalizeLayering();
+ VERIFY_IS_FALSE(newLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk");
+ const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
+ const auto newResult{ newSettings->ToJson() };
+
+ VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
+ }
+
+ void SerializationTests::MultipleActionsAreCollapsed()
+ {
+ static constexpr std::string_view oldSettingsJson{ R"(
+ {
+ "actions": [
+ {
+ "name": "foo",
+ "icon": "myCoolIconPath.png",
+ "command": { "action": "sendInput", "input": "just some input" },
+ "keys": "ctrl+shift+w"
+ },
+ {
+ "command": { "action": "sendInput", "input": "just some input" },
+ "keys": "ctrl+shift+x"
+ }
+ ]
+ })" };
+
+ // modern style:
+ // - multiple action blocks whose purpose is simply to define more keybindings for the same action
+ // get collapsed into one action block, with the name and iconpath preserved and have multiple keybindings instead
+ static constexpr std::string_view newSettingsJson{ R"(
+ {
+ "actions": [
+ {
+ "name": "foo",
+ "icon": "myCoolIconPath.png",
+ "command": { "action": "sendInput", "input": "just some input" },
+ "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"("
+ }
+ ],
+ "keybindings": [
+ {
+ "keys": "ctrl+shift+w",
+ "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"("
+ },
+ {
+ "keys": "ctrl+shift+x",
+ "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"("
+ }
+ ]
+ })" };
+
+ implementation::SettingsLoader loader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
+ loader.MergeInboxIntoUserSettings();
+ loader.FinalizeLayering();
+ VERIFY_IS_TRUE(loader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
+ const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
+ const auto oldResult{ settings->ToJson() };
+
+ implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
+ newLoader.MergeInboxIntoUserSettings();
+ newLoader.FinalizeLayering();
+ VERIFY_IS_FALSE(newLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk");
+ const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
+ const auto newResult{ newSettings->ToJson() };
+
+ VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
+ }
 }