[sync] Allow custom debug messages in AwaitMatchStatusChangeChecker

There's no reason for this class to take a fixed string with a debug
message upon construction. It can instead produce the string dynamically
upon condition verification, such that the failure messages provide a
better insight into the actual state.

Bug: 1150829

Change-Id: I693339cc1bfe79ce69c28d7ce84faf8e48ec3562
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2557986Reviewed-by: Maksim Moskvitin <mmoskvitin@google.com>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831088}

chrome/browser/sync/test/integration/await_match_status_change_checker.cc

@@ -8,18 +8,14 @@
 #include "chrome/browser/sync/test/integration/sync_test.h"
 
 AwaitMatchStatusChangeChecker::AwaitMatchStatusChangeChecker(
-    const ExitConditionCallback& condition,
-    const std::string& debug_message)
+    const ExitConditionCallback& condition)
     : MultiClientStatusChangeChecker(
           sync_datatype_helper::test()->GetSyncServices()),
-      condition_(condition),
-      debug_message_(debug_message) {
-}
+      condition_(condition) {}
 
 AwaitMatchStatusChangeChecker::~AwaitMatchStatusChangeChecker() {
 }
 
 bool AwaitMatchStatusChangeChecker::IsExitConditionSatisfied(std::ostream* os) {
-  *os << "Waiting for: " + debug_message_;
-  return condition_.Run();
+  return condition_.Run(os);
 }

chrome/browser/sync/test/integration/await_match_status_change_checker.h

@@ -5,7 +5,7 @@
 #ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_AWAIT_MATCH_STATUS_CHANGE_CHECKER_H_
 #define CHROME_BROWSER_SYNC_TEST_INTEGRATION_AWAIT_MATCH_STATUS_CHANGE_CHECKER_H_
 
-#include <string>
+#include <iosfwd>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ -16,17 +16,17 @@
 // verifies the state of all the profiles involved in the test.
 class AwaitMatchStatusChangeChecker : public MultiClientStatusChangeChecker {
  public:
-  using ExitConditionCallback = base::RepeatingCallback<bool(void)>;
+  // The std::ostream allows the callback to output debug messages.
+  using ExitConditionCallback = base::RepeatingCallback<bool(std::ostream*)>;
 
-  AwaitMatchStatusChangeChecker(const ExitConditionCallback& condition,
-                                const std::string& debug_message);
+  explicit AwaitMatchStatusChangeChecker(
+      const ExitConditionCallback& condition);
   ~AwaitMatchStatusChangeChecker() override;
 
   bool IsExitConditionSatisfied(std::ostream* os) override;
 
  private:
-  ExitConditionCallback condition_;
-  std::string debug_message_;
+  const ExitConditionCallback condition_;
 };
 
 #endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_AWAIT_MATCH_STATUS_CHANGE_CHECKER_H_

chrome/browser/sync/test/integration/printers_helper.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/sync/test/integration/printers_helper.h"
 
 #include <algorithm>
+#include <ostream>
 #include <string>
 #include <unordered_map>
 #include <utility>
@@ -153,12 +154,14 @@ int GetPrinterCount(int index) {
   return GetPrinterStore(index)->GetSavedPrinters().size();
 }
 
-bool AllProfilesContainSamePrinters() {
+bool AllProfilesContainSamePrinters(std::ostream* os) {
   auto reference_printers = GetPrinterStore(0)->GetSavedPrinters();
   for (int i = 1; i < test()->num_clients(); ++i) {
     auto printers = GetPrinterStore(i)->GetSavedPrinters();
     if (!ListsContainTheSamePrinters(reference_printers, printers)) {
-      VLOG(1) << "Printers in client [" << i << "] don't match client 0";
+      if (os) {
+        *os << "Printers in client [" << i << "] don't match client 0";
+      }
       return false;
     }
   }
@@ -173,9 +176,8 @@ bool ProfileContainsSamePrintersAsVerifier(int index) {
 }
 
 PrintersMatchChecker::PrintersMatchChecker()
-    : AwaitMatchStatusChangeChecker(
-          base::BindRepeating(&printers_helper::AllProfilesContainSamePrinters),
-          "All printers match") {}
+    : AwaitMatchStatusChangeChecker(base::BindRepeating(
+          &printers_helper::AllProfilesContainSamePrinters)) {}
 
 PrintersMatchChecker::~PrintersMatchChecker() {}
 

chrome/browser/sync/test/integration/printers_helper.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_PRINTERS_HELPER_H_
 #define CHROME_BROWSER_SYNC_TEST_INTEGRATION_PRINTERS_HELPER_H_
 
+#include <iosfwd>
 #include <memory>
 #include <string>
 
@@ -58,7 +59,7 @@ int GetVerifierPrinterCount();
 int GetPrinterCount(int index);
 
 // Returns true if all profiles contain the same printers as profile 0.
-bool AllProfilesContainSamePrinters();
+bool AllProfilesContainSamePrinters(std::ostream* os = nullptr);
 
 // Returns true if the verifier store and printer store |index| contain the same
 // data.

chrome/browser/sync/test/integration/single_client_extensions_sync_test.cc

@@ -65,8 +65,13 @@ IN_PROC_BROWSER_TEST_F(SingleClientExtensionsSyncTest, InstallSomeExtensions) {
 
 // Helper function for waiting to see the extension count in a profile
 // become a specific number.
-static bool ExtensionCountCheck(Profile* profile, size_t expected_count) {
-  return GetInstalledExtensions(profile).size() == expected_count;
+static bool ExtensionCountCheck(Profile* profile,
+                                size_t expected_count,
+                                std::ostream* os) {
+  const size_t actual_count = GetInstalledExtensions(profile).size();
+  *os << "Waiting for profile to have " << expected_count
+      << " extensions; actual count " << actual_count;
+  return actual_count == expected_count;
 }
 
 // Tests the case of an uninstall from the server conflicting with a local
@@ -94,9 +99,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientExtensionsSyncTest, UninstallWinsConflicts) {
   ASSERT_EQ(1u, GetInstalledExtensions(GetProfile(0)).size());
 
   // Expect the extension to get uninstalled locally.
-  AwaitMatchStatusChangeChecker checker(
-      base::BindRepeating(&ExtensionCountCheck, GetProfile(0), 0u),
-      "Waiting for profile to have no extensions");
+  AwaitMatchStatusChangeChecker checker(base::BindRepeating(
+      &ExtensionCountCheck, GetProfile(0), /*expected_count=*/0u));
   EXPECT_TRUE(checker.Wait());
   EXPECT_TRUE(GetInstalledExtensions(GetProfile(0)).empty());