Make test outputs involving PasswordForms more readable.

components/password_manager/core/browser/password_manager_test_utils.h

Index: components/password_manager/core/browser/password_manager_test_utils.h
diff --git a/components/password_manager/core/browser/password_manager_test_utils.h b/components/password_manager/core/browser/password_manager_test_utils.h
index 71599a446b8a0dfab35e83201e9b1c91741b5fd1..b1600bdbf5dad3204c273b4da2354f008a698994 100644
--- a/components/password_manager/core/browser/password_manager_test_utils.h
+++ b/components/password_manager/core/browser/password_manager_test_utils.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_TEST_UTILS_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_TEST_UTILS_H_
 
+#include <iosfwd>
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ -42,17 +43,19 @@ struct PasswordFormData {
 scoped_ptr<autofill::PasswordForm> CreatePasswordFormFromDataForTesting(
     const PasswordFormData& form_data);
 
-// Checks whether two vectors of PasswordForms contain equivalent elements,
-// regardless of order.
-// TODO(vabr) The *Ptr suffix is obsolete, rename.
-bool ContainsSamePasswordFormsPtr(
-    const std::vector<autofill::PasswordForm*>& first,
-    const std::vector<autofill::PasswordForm*>& second);
-
-// This gmock matcher is used to check that the |arg| contains exactly the same
-// PasswordForms as |forms|, regardless of order.
-MATCHER_P(ContainsSamePasswordForms, forms, "") {
-  return ContainsSamePasswordFormsPtr(forms, arg);
+// Checks whether |actual_values| contains pointers to PasswordForms that are in
+// some order pairwise equal to those in |expectations|. Details of mismatches
+// will be written in human readable form to |mismatches_output| if not NULL.

[vabr (Chromium), 2015/03/19 13:28:12]

optional nit: "if not NULL" -> "unless it is NULL".
These are 2 proposed changes:
(1) NULL (which is a deprecated C++ language constant) -> null (an English
adjective)
(2) introducing "it is" to make clear that it is |mismatches_output| which are
potentially null, not the details.

[engedy, 2015/03/20 13:17:15]

Both done.

+bool ContainsEqualPasswordFormsUnordered(
+    const std::vector<autofill::PasswordForm*>& expectations,
+    const std::vector<autofill::PasswordForm*>& actual_values,
+    std::ostream* mismatches_output);
+
+// GMock matcher that matches an |arg| containing pointers to PasswordForms that
+// are equal to those in |forms|, regardless of order.

[vabr (Chromium), 2015/03/19 13:28:12]

forms -> expected_forms

[vabr (Chromium), 2015/03/19 13:28:12]

"equal to those in |forms|" sounds a bit incorrect, as it can be misinterpreted
as "equal to every single form in |forms|".

Also, it is not exactly clear, if this is in the set or multiset sense. The fact
a form can be represented twice because of both the same pointer appearing
twice, or two different pointers pointing to identical copies of a PasswordForm,
does not help with explaining this.

Would you mind trying to make the description a bit more precise in specifying
which equality (set vs. multiset) it checks, and what is assumed about the
expected_forms (e.g., not pointer duplicities, etc.)?

[engedy, 2015/03/20 13:17:15]

N/A anymore.

[engedy, 2015/03/20 13:17:15]

Done. I have updated the comment above, and completely removed it for the GMock
matcher -- I do not think it adds much value.

+MATCHER_P(UnorderedPasswordFormElementsAre, expected_forms, "") {
+  return ContainsEqualPasswordFormsUnordered(expected_forms, arg,
+                                             result_listener->stream());
 }
 
 }  // namespace password_manager