Merge pull request #13676 from LillyJadeKatrin/retroachievements-allowlist-test-improvements

RetroAchievements: Updated PatchAllowlistTest to generate new allowlist

Source/Core/Core/AchievementManager.h

@@ -89,8 +89,8 @@ public:
   static constexpr std::string_view BLUE = "#0B71C1";
   static constexpr std::string_view APPROVED_LIST_FILENAME = "ApprovedInis.json";
   static const inline Common::SHA1::Digest APPROVED_LIST_HASH = {
-      0xE1, 0x29, 0xD1, 0x33, 0x4D, 0xF2, 0xF8, 0xA8, 0x4E, 0xCA,
+      0x6D, 0x91, 0xF5, 0xC1, 0xE2, 0x4C, 0xC3, 0x39, 0xF5, 0x7F,
-      0xF6, 0x87, 0xE6, 0xEC, 0xEC, 0xB3, 0x18, 0x69, 0x34, 0x45};
+      0xEC, 0xA9, 0x8C, 0xA9, 0xBD, 0x61, 0x28, 0x54, 0x11, 0x62};
 
   struct LeaderboardEntry
   {

Source/UnitTests/Core/PatchAllowlistTest.cpp

@@ -18,6 +18,7 @@
 #include "Common/IOFile.h"
 #include "Common/IniFile.h"
 #include "Common/JsonUtil.h"
+#include "Core/AchievementManager.h"
 #include "Core/ActionReplay.h"
 #include "Core/CheatCodes.h"
 #include "Core/GeckoCode.h"
@@ -35,46 +36,23 @@ using AllowList = std::map<std::string /*ID*/, GameHashes>;
 template <typename T>
 void ReadVerified(const Common::IniFile& ini, const std::string& filename,
                   const std::string& section, bool enabled, std::vector<T>* codes);
-void CheckHash(const std::string& game_id, GameHashes* game_hashes, const std::string& hash,
-               const std::string& patch_name);
 
 TEST(PatchAllowlist, VerifyHashes)
 {
-  // Load allowlist
+  // Iterate over GameSettings directory
-  static constexpr std::string_view APPROVED_LIST_FILENAME = "ApprovedInis.json";
+  picojson::object new_allowlist;
-  picojson::value json_tree;
-  std::string error;
   std::string cur_directory = File::GetExeDirectory()
 #if defined(__APPLE__)
                               + DIR_SEP "Tests"  // FIXME: Ugly hack.
 #endif
       ;
   std::string sys_directory = cur_directory + DIR_SEP "Sys";
-  const auto& list_filepath = fmt::format("{}{}{}", sys_directory, DIR_SEP, APPROVED_LIST_FILENAME);
-  ASSERT_TRUE(JsonFromFile(list_filepath, &json_tree, &error))
-      << "Failed to open file at " << list_filepath;
-  // Parse allowlist - Map<game id, Map<hash, name>>
-  ASSERT_TRUE(json_tree.is<picojson::object>());
-  AllowList allow_list;
-  for (const auto& entry : json_tree.get<picojson::object>())
-  {
-    ASSERT_TRUE(entry.second.is<picojson::object>());
-    GameHashes& game_entry = allow_list[entry.first];
-    for (const auto& line : entry.second.get<picojson::object>())
-    {
-      ASSERT_TRUE(line.second.is<std::string>());
-      if (line.first == "title")
-        game_entry.game_title = line.second.get<std::string>();
-      else
-        game_entry.hashes[line.first] = line.second.get<std::string>();
-    }
-  }
-  // Iterate over GameSettings directory
   auto directory =
       File::ScanDirectoryTree(fmt::format("{}{}GameSettings", sys_directory, DIR_SEP), false);
   for (const auto& file : directory.children)
   {
     // Load ini file
+    picojson::object approved;
     Common::IniFile ini_file;
     ini_file.Load(file.physicalName, true);
     std::string game_id = file.virtualName.substr(0, file.virtualName.find_first_of('.'));
@@ -90,9 +68,6 @@ TEST(PatchAllowlist, VerifyHashes)
                                    &geckos);
     ReadVerified<ActionReplay::ARCode>(ini_file, game_id, "AR_RetroAchievements_Verified", true,
                                        &action_replays);
-    // Get game section from allow list
-    auto game_itr = allow_list.find(game_id);
-    bool itr_end = (game_itr == allow_list.end());
     // Iterate over approved patches
     for (const auto& patch : patches)
     {
@@ -110,8 +85,7 @@ TEST(PatchAllowlist, VerifyHashes)
         context->Update(Common::BitCastToArray<u8>(entry.conditional));
       }
       auto digest = context->Finish();
-      CheckHash(game_id, itr_end ? nullptr : &game_itr->second,
+      approved[patch.name] = picojson::value(Common::SHA1::DigestToString(digest));
-                Common::SHA1::DigestToString(digest), patch.name);
     }
     // Iterate over approved geckos
     for (const auto& code : geckos)
@@ -127,8 +101,7 @@ TEST(PatchAllowlist, VerifyHashes)
         context->Update(Common::BitCastToArray<u8>(entry.data));
       }
       auto digest = context->Finish();
-      CheckHash(game_id, itr_end ? nullptr : &game_itr->second,
+      approved[code.name] = picojson::value(Common::SHA1::DigestToString(digest));
-                Common::SHA1::DigestToString(digest), code.name);
     }
     // Iterate over approved AR codes
     for (const auto& code : action_replays)
@@ -144,27 +117,43 @@ TEST(PatchAllowlist, VerifyHashes)
         context->Update(Common::BitCastToArray<u8>(entry.value));
       }
       auto digest = context->Finish();
-      CheckHash(game_id, itr_end ? nullptr : &game_itr->second,
+      approved[code.name] = picojson::value(Common::SHA1::DigestToString(digest));
-                Common::SHA1::DigestToString(digest), code.name);
     }
-    // Report missing patches in map
+    // Add approved patches and codes to tree
-    if (itr_end)
+    if (!approved.empty())
-      continue;
+      new_allowlist[game_id] = picojson::value(approved);
-    for (auto& remaining_hashes : game_itr->second.hashes)
+  }
+
+  // Hash new allowlist
+  std::string new_allowlist_str = picojson::value(new_allowlist).serialize();
+  auto context = Common::SHA1::CreateContext();
+  context->Update(new_allowlist_str);
+  auto digest = context->Finish();
+  if (digest != AchievementManager::APPROVED_LIST_HASH)
   {
-      ADD_FAILURE() << "Hash in list not approved in ini." << std::endl
+    ADD_FAILURE() << "Approved list hash does not match the one in AchievementMananger."
-                    << "Game ID: " << game_id << ":" << game_itr->second.game_title << std::endl
+                  << std::endl
-                    << "Code: " << remaining_hashes.first << ":" << remaining_hashes.second;
+                  << "Please update APPROVED_LIST_HASH to the following:" << std::endl
+                  << Common::SHA1::DigestToString(digest);
   }
-    //    Remove section from map
+  // Compare with old allowlist
-    allow_list.erase(game_itr);
+  static constexpr std::string_view APPROVED_LIST_FILENAME = "ApprovedInis.json";
-  }
+  std::string old_allowlist;
-  //  Report remaining sections in map
+  std::string error;
-  for (auto& remaining_games : allow_list)
+  const auto& list_filepath = fmt::format("{}{}{}", sys_directory, DIR_SEP, APPROVED_LIST_FILENAME);
+  if (!File::ReadFileToString(list_filepath, old_allowlist) || old_allowlist != new_allowlist_str)
   {
-    ADD_FAILURE() << "Game in list has no ini file." << std::endl
+    static constexpr std::string_view NEW_APPROVED_LIST_FILENAME = "New-ApprovedInis.json";
-                  << "Game ID: " << remaining_games.first << ":"
+    const auto& new_list_filepath =
-                  << remaining_games.second.game_title;
+        fmt::format("{}{}{}", sys_directory, DIR_SEP, NEW_APPROVED_LIST_FILENAME);
+    if (!JsonToFile(new_list_filepath, picojson::value(new_allowlist), false))
+    {
+      ADD_FAILURE() << "Failed to write new approved list to " << list_filepath;
+    }
+    ADD_FAILURE() << "Approved list needs to be updated. Please run this test in your" << std::endl
+                  << "local environment and copy" << std::endl
+                  << new_list_filepath << std::endl
+                  << "to Data/Sys/ApprovedInis.json to pass this test.";
   }
 }
 
@@ -202,30 +191,3 @@ void ReadVerified(const Common::IniFile& ini, const std::string& filename,
     }
   }
 }
-
-void CheckHash(const std::string& game_id, GameHashes* game_hashes, const std::string& hash,
-               const std::string& patch_name)
-{
-  // Check patch in list
-  if (game_hashes == nullptr)
-  {
-    // Report: no patches in game found in list
-    ADD_FAILURE() << "Approved hash missing from list." << std::endl
-                  << "Game ID: " << game_id << std::endl
-                  << "Code: \"" << hash << "\": \"" << patch_name << "\"";
-    return;
-  }
-  auto hash_itr = game_hashes->hashes.find(hash);
-  if (hash_itr == game_hashes->hashes.end())
-  {
-    // Report: patch not found in list
-    ADD_FAILURE() << "Approved hash missing from list." << std::endl
-                  << "Game ID: " << game_id << ":" << game_hashes->game_title << std::endl
-                  << "Code: \"" << hash << "\": \"" << patch_name << "\"";
-  }
-  else
-  {
-    // Remove patch from map if found
-    game_hashes->hashes.erase(hash_itr);
-  }
-}

Source/UnitTests/UnitTests.vcxproj

@@ -104,6 +104,7 @@
   <Import Project="$(ExternalsDir)Bochs_disasm\exports.props" />
   <Import Project="$(ExternalsDir)fmt\exports.props" />
   <Import Project="$(ExternalsDir)picojson\exports.props" />
+  <Import Project="$(ExternalsDir)rcheevos\exports.props" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>