Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1597)

Unified Diff: chrome/browser/component_updater/sw_reporter_installer_win_unittest.cc

Issue 2226133005: Add support for the ExperimentalSwReporterEngine field trial. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix case of flags constants Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/component_updater/sw_reporter_installer_win_unittest.cc
diff --git a/chrome/browser/component_updater/sw_reporter_installer_win_unittest.cc b/chrome/browser/component_updater/sw_reporter_installer_win_unittest.cc
index 0c8cb199a4c40882c2e7fc1d2137c3a0aa18af6b..c957a80cf93669c45b5ee5dc7d8ac1b3fb75d274 100644
--- a/chrome/browser/component_updater/sw_reporter_installer_win_unittest.cc
+++ b/chrome/browser/component_updater/sw_reporter_installer_win_unittest.cc
@@ -37,6 +37,8 @@ constexpr char kErrorHistogramName[] = "SoftwareReporter.ExperimentErrors";
constexpr char kExperimentTag[] = "experiment_tag";
constexpr char kMissingTag[] = "missing_tag";
+using safe_browsing::SwReporterInvocation;
+
} // namespace
class SwReporterInstallerTest : public ::testing::Test {
@@ -46,15 +48,20 @@ class SwReporterInstallerTest : public ::testing::Test {
base::Bind(&SwReporterInstallerTest::SwReporterLaunched,
base::Unretained(this))),
default_version_("1.2.3"),
- default_path_(L"C:\\full\\path\\to\\download") {}
+ default_path_(L"C:\\full\\path\\to\\download"),
+ launched_version_("0.0.0") {}
~SwReporterInstallerTest() override {}
protected:
- void SwReporterLaunched(const safe_browsing::SwReporterInvocation& invocation,
+ void SwReporterLaunched(const safe_browsing::SwReporterQueue& invocations,
const base::Version& version) {
ASSERT_TRUE(launched_invocations_.empty());
- launched_invocations_.push_back(invocation);
+ auto mutable_invocations = invocations;
Sorin Jianu 2016/09/01 15:59:09 Can we use an algorithm to filter out items instea
Joe Mason 2016/09/02 02:24:30 You can't use an algorithm on std::queue because i
+ while (!mutable_invocations.empty()) {
+ launched_invocations_.push_back(mutable_invocations.front());
+ mutable_invocations.pop();
+ }
launched_version_ = version;
}
@@ -73,14 +80,16 @@ class SwReporterInstallerTest : public ::testing::Test {
EXPECT_EQ(default_version_, launched_version_);
ASSERT_EQ(1U, launched_invocations_.size());
- const safe_browsing::SwReporterInvocation& invocation =
- launched_invocations_[0];
+ const SwReporterInvocation& invocation = launched_invocations_[0];
EXPECT_EQ(MakeTestFilePath(default_path_),
invocation.command_line.GetProgram());
EXPECT_TRUE(invocation.command_line.GetSwitches().empty());
EXPECT_TRUE(invocation.command_line.GetArgs().empty());
EXPECT_TRUE(invocation.suffix.empty());
- EXPECT_FALSE(invocation.is_experimental);
+ EXPECT_EQ(SwReporterInvocation::FLAG_LOG_TO_RAPPOR |
+ SwReporterInvocation::FLAG_LOG_TO_PREFS |
+ SwReporterInvocation::FLAG_TRIGGER_PROMPT,
+ invocation.flags);
}
// |ComponentReady| asserts that it is run on the UI thread, so we must
@@ -95,7 +104,7 @@ class SwReporterInstallerTest : public ::testing::Test {
base::FilePath default_path_;
// Results of running |ComponentReady|.
- std::vector<safe_browsing::SwReporterInvocation> launched_invocations_;
+ std::vector<SwReporterInvocation> launched_invocations_;
base::Version launched_version_;
private:
@@ -161,8 +170,7 @@ class ExperimentalSwReporterInstallerTest : public SwReporterInstallerTest {
EXPECT_EQ(default_version_, launched_version_);
ASSERT_EQ(1U, launched_invocations_.size());
- const safe_browsing::SwReporterInvocation& invocation =
- launched_invocations_[0];
+ const SwReporterInvocation& invocation = launched_invocations_[0];
EXPECT_EQ(MakeTestFilePath(default_path_),
invocation.command_line.GetProgram());
@@ -184,7 +192,7 @@ class ExperimentalSwReporterInstallerTest : public SwReporterInstallerTest {
invocation.command_line.GetArgs()[0]);
}
- EXPECT_TRUE(invocation.is_experimental);
+ EXPECT_EQ(0, invocation.flags);
histograms_.ExpectTotalCount(kErrorHistogramName, 0);
}
@@ -274,7 +282,8 @@ TEST_F(ExperimentalSwReporterInstallerTest, SingleInvocation) {
"{\"launch_params\": ["
" {"
" \"arguments\": [\"--engine=experimental\", \"random argument\"],"
- " \"suffix\": \"TestSuffix\""
+ " \"suffix\": \"TestSuffix\","
+ " \"prompt\": false"
" }"
"]}";
traits.ComponentReady(
@@ -285,8 +294,7 @@ TEST_F(ExperimentalSwReporterInstallerTest, SingleInvocation) {
EXPECT_EQ(default_version_, launched_version_);
ASSERT_EQ(1U, launched_invocations_.size());
- const safe_browsing::SwReporterInvocation& invocation =
- launched_invocations_[0];
+ const SwReporterInvocation& invocation = launched_invocations_[0];
EXPECT_EQ(MakeTestFilePath(default_path_),
invocation.command_line.GetProgram());
EXPECT_EQ(2U, invocation.command_line.GetSwitches().size());
@@ -297,7 +305,7 @@ TEST_F(ExperimentalSwReporterInstallerTest, SingleInvocation) {
ASSERT_EQ(1U, invocation.command_line.GetArgs().size());
EXPECT_EQ(L"random argument", invocation.command_line.GetArgs()[0]);
EXPECT_EQ("TestSuffix", invocation.suffix);
- EXPECT_TRUE(invocation.is_experimental);
+ EXPECT_EQ(0, invocation.flags);
histograms_.ExpectTotalCount(kErrorHistogramName, 0);
}
@@ -310,19 +318,70 @@ TEST_F(ExperimentalSwReporterInstallerTest, MultipleInvocations) {
"{\"launch_params\": ["
" {"
" \"arguments\": [\"--engine=experimental\", \"random argument\"],"
- " \"suffix\": \"TestSuffix\""
+ " \"suffix\": \"TestSuffix\","
+ " \"prompt\": false"
" },"
" {"
" \"arguments\": [\"--engine=second\"],"
- " \"suffix\": \"SecondSuffix\""
+ " \"suffix\": \"SecondSuffix\","
+ " \"prompt\": true"
+ " },"
+ " {"
+ " \"arguments\": [\"--engine=third\"],"
+ " \"suffix\": \"ThirdSuffix\""
" }"
"]}";
traits.ComponentReady(
default_version_, default_path_,
base::DictionaryValue::From(base::JSONReader::Read(kTestManifest)));
- // Not supported yet.
- ExpectLaunchError();
+ // The SwReporter should be launched three times with the given arguments.
+ EXPECT_EQ(default_version_, launched_version_);
+ ASSERT_EQ(3U, launched_invocations_.size());
+
+ {
+ const SwReporterInvocation& invocation = launched_invocations_[0];
+ EXPECT_EQ(MakeTestFilePath(default_path_),
+ invocation.command_line.GetProgram());
+ EXPECT_EQ(2U, invocation.command_line.GetSwitches().size());
+ EXPECT_EQ("experimental",
+ invocation.command_line.GetSwitchValueASCII("engine"));
+ EXPECT_EQ("TestSuffix", invocation.command_line.GetSwitchValueASCII(
+ kRegistrySuffixSwitch));
+ ASSERT_EQ(1U, invocation.command_line.GetArgs().size());
+ EXPECT_EQ(L"random argument", invocation.command_line.GetArgs()[0]);
+ EXPECT_EQ("TestSuffix", invocation.suffix);
+ EXPECT_EQ(0, invocation.flags);
+ }
+
+ {
+ const SwReporterInvocation& invocation = launched_invocations_[1];
+ EXPECT_EQ(MakeTestFilePath(default_path_),
+ invocation.command_line.GetProgram());
+ EXPECT_EQ(2U, invocation.command_line.GetSwitches().size());
+ EXPECT_EQ("second", invocation.command_line.GetSwitchValueASCII("engine"));
+ EXPECT_EQ("SecondSuffix", invocation.command_line.GetSwitchValueASCII(
+ kRegistrySuffixSwitch));
+ ASSERT_TRUE(invocation.command_line.GetArgs().empty());
+ EXPECT_EQ("SecondSuffix", invocation.suffix);
+ EXPECT_EQ(SwReporterInvocation::FLAG_TRIGGER_PROMPT, invocation.flags);
+ }
+
+ {
+ const SwReporterInvocation& invocation = launched_invocations_[2];
+ EXPECT_EQ(MakeTestFilePath(default_path_),
+ invocation.command_line.GetProgram());
+ EXPECT_EQ(2U, invocation.command_line.GetSwitches().size());
+ EXPECT_EQ("third", invocation.command_line.GetSwitchValueASCII("engine"));
+ EXPECT_EQ("ThirdSuffix", invocation.command_line.GetSwitchValueASCII(
+ kRegistrySuffixSwitch));
+ ASSERT_TRUE(invocation.command_line.GetArgs().empty());
+ EXPECT_EQ("ThirdSuffix", invocation.suffix);
+ // A missing "prompt" key means "false".
+ EXPECT_EQ(0, invocation.flags);
+ }
+
+ histograms_.ExpectTotalCount(kErrorHistogramName, 0);
}
TEST_F(ExperimentalSwReporterInstallerTest, MissingSuffix) {
@@ -623,4 +682,26 @@ TEST_F(ExperimentalSwReporterInstallerTest, BadTypesInManifest3) {
SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS, 1);
}
+TEST_F(ExperimentalSwReporterInstallerTest, BadTypesInManifest4) {
+ SwReporterInstallerTraits traits(launched_callback_, true);
+ CreateFeatureWithTag(kExperimentTag);
+
+ // This has an int instead of a bool for prompt.
+ static constexpr char kTestManifest[] =
+ "{\"launch_params\": ["
+ " {"
+ " \"arguments\": [\"--engine=experimental\"],"
+ " \"suffix\": \"TestSuffix\","
+ " \"prompt\": 1"
+ " }"
+ "]}";
+ traits.ComponentReady(
+ default_version_, default_path_,
+ base::DictionaryValue::From(base::JSONReader::Read(kTestManifest)));
+
+ // The SwReporter should not be launched, and an error should be logged.
+ EXPECT_TRUE(launched_invocations_.empty());
+ histograms_.ExpectUniqueSample(kErrorHistogramName,
+ SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS, 1);
+}
} // namespace component_updater

Powered by Google App Engine
This is Rietveld 408576698