Allow PolicyLoadStatusSample to override reporting method

components/policy/core/common/preg_parser_unittest.cc

Index: components/policy/core/common/preg_parser_unittest.cc
diff --git a/components/policy/core/common/preg_parser_unittest.cc b/components/policy/core/common/preg_parser_unittest.cc
index d45a57bd7d13ea1f1d3cec9fda597830b3cca091..1c4e92134dc5b6bea8e582f0d2eb06bc37a5ffc6 100644
--- a/components/policy/core/common/preg_parser_unittest.cc
+++ b/components/policy/core/common/preg_parser_unittest.cc
@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/base_paths.h"
+#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ -26,6 +27,7 @@ namespace {
 const char kRegistryPolBaseDir[] = "chrome/test/data/policy/gpo";
 const char kRegistryPolFile[] = "parser_test/registry.pol";
 const char kInvalidEncodingRegistryPolFile[] = "invalid_encoding/registry.pol";
+const char kNonExistingRegistryPolFile[] = "does_not_exist.pol";
 
 const char kRegistryKey[] = "SOFTWARE\\Policies\\Chromium";
 
@@ -116,7 +118,7 @@ TEST_F(PRegParserTest, TestParseFile) {
 
   // Run the parser.
   base::FilePath test_file(test_data_dir_.AppendASCII(kRegistryPolFile));
-  PolicyLoadStatusSample status;
+  PolicyLoadStatusAutoSubmitter status;
   ASSERT_TRUE(preg_parser::ReadFile(test_file, base::ASCIIToUTF16(kRegistryKey),
                                     &dict, &status));
 
@@ -144,7 +146,7 @@ TEST_F(PRegParserTest, SubstringRootInvalid) {
   // key like "Aa/Bb/C".
   base::FilePath test_file(test_data_dir_.AppendASCII(kRegistryPolFile));
   RegistryDict empty;
-  PolicyLoadStatusSample status;
+  PolicyLoadStatusAutoSubmitter status;
 
   // No data should be loaded for partial roots ("Aa/Bb/C").
   RegistryDict dict1;
@@ -164,7 +166,7 @@ TEST_F(PRegParserTest, RejectInvalidStrings) {
   // Tests whether strings with invalid characters are rejected.
   base::FilePath test_file(
       test_data_dir_.AppendASCII(kInvalidEncodingRegistryPolFile));
-  PolicyLoadStatusSample status;
+  PolicyLoadStatusAutoSubmitter status;
   RegistryDict dict;
   ASSERT_TRUE(preg_parser::ReadFile(test_file, base::ASCIIToUTF16(kRegistryKey),
                                     &dict, &status));
@@ -173,6 +175,21 @@ TEST_F(PRegParserTest, RejectInvalidStrings) {
   EXPECT_TRUE(RegistryDictEquals(dict, empty));
 }
 
+TEST_F(PRegParserTest, OverrideReportCallback) {

[emaxx, 2017/05/05 10:44:25]

nit: Update the test name - there's no callback now.

[ljusten (tachyonic), 2017/05/05 12:58:36]

Done.

+  // Tests load status sampling.
+  PolicyLoadStatusAutoSubmitter status;

[emaxx, 2017/05/05 10:44:25]

Probably testing the PolicyLoadStatusSampler class is more appropriate here? As
this test needs only GetStatusSet().
(BTW, I would prefer compositing to inheritance of classes - then such misplace
won't be possible. But not insisting.)

[ljusten (tachyonic), 2017/05/05 12:58:36]

Wasn't sure which one I should use. On the one hand, it makes sense to test
what's used in prod, on the other hand it's not necessary here. I'll keep anyway
it since this is used in prod.
I didn't want to change all locations where status is passed as parameter. If I
used composition, then I'd probably embed the sampler in the reporter and have a
getter and call reporter->GetSampler() or something like that instead of passing
in the status.

+  RegistryDict dict;
+  base::FilePath test_file(
+      test_data_dir_.AppendASCII(kNonExistingRegistryPolFile));
+  ASSERT_FALSE(preg_parser::ReadFile(
+      test_file, base::ASCIIToUTF16(kRegistryKey), &dict, &status));
+
+  PolicyLoadStatusSampler::StatusSet expected_status_set;
+  expected_status_set[POLICY_LOAD_STATUS_STARTED] = true;
+  expected_status_set[POLICY_LOAD_STATUS_READ_ERROR] = true;
+  EXPECT_EQ(expected_status_set, status.GetStatusSet());
+}
+
 }  // namespace
 }  // namespace preg_parser
 }  // namespace policy