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

RetroAchievements: Updated PatchAllowlistTest to generate new allowlist

1 files changed, 40 insertions, 78 deletions

diff --git a/Source/UnitTests/Core/PatchAllowlistTest.cpp b/Source/UnitTests/Core/PatchAllowlistTest.cpp
index 2aae282216..c2b6be61e6 100644
--- a/Source/UnitTests/Core/PatchAllowlistTest.cpp
+++ b/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
-  static constexpr std::string_view APPROVED_LIST_FILENAME = "ApprovedInis.json";
-  picojson::value json_tree;
-  std::string error;
+  // Iterate over GameSettings directory
+  picojson::object new_allowlist;
   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,
-                Common::SHA1::DigestToString(digest), patch.name);
+      approved[patch.name] = picojson::value(Common::SHA1::DigestToString(digest));
     }
     // 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,
-                Common::SHA1::DigestToString(digest), code.name);
+      approved[code.name] = picojson::value(Common::SHA1::DigestToString(digest));
     }
     // 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,
-                Common::SHA1::DigestToString(digest), code.name);
-    }
-    // Report missing patches in map
-    if (itr_end)
-      continue;
-    for (auto& remaining_hashes : game_itr->second.hashes)
-    {
-      ADD_FAILURE() << "Hash in list not approved in ini." << std::endl
-                    << "Game ID: " << game_id << ":" << game_itr->second.game_title << std::endl
-                    << "Code: " << remaining_hashes.first << ":" << remaining_hashes.second;
+      approved[code.name] = picojson::value(Common::SHA1::DigestToString(digest));
     }
-    //    Remove section from map
-    allow_list.erase(game_itr);
+    // Add approved patches and codes to tree
+    if (!approved.empty())
+      new_allowlist[game_id] = picojson::value(approved);
   }
-  //  Report remaining sections in map
-  for (auto& remaining_games : allow_list)
+
+  // 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() << "Game in list has no ini file." << std::endl
-                  << "Game ID: " << remaining_games.first << ":"
-                  << remaining_games.second.game_title;
+    ADD_FAILURE() << "Approved list hash does not match the one in AchievementMananger."
+                  << std::endl
+                  << "Please update APPROVED_LIST_HASH to the following:" << std::endl
+                  << Common::SHA1::DigestToString(digest);
+  }
+  // Compare with old allowlist
+  static constexpr std::string_view APPROVED_LIST_FILENAME = "ApprovedInis.json";
+  std::string old_allowlist;
+  std::string error;
+  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)
+  {
+    static constexpr std::string_view NEW_APPROVED_LIST_FILENAME = "New-ApprovedInis.json";
+    const auto& new_list_filepath =
+        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);
-  }
-}