Omnibox: Create Bundled Field Trial; Convert SearchHistory trial to it

chrome/browser/omnibox/omnibox_field_trial_unittest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 
 #include "base/basictypes.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string16.h"
 #include "chrome/common/metrics/entropy_provider.h"
+#include "chrome/common/metrics/variations/variations_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class OmniboxFieldTrialTest : public testing::Test {
  public:
   OmniboxFieldTrialTest() {}
 
   static void SetUpTestCase() {
     ResetFieldTrialList();
   }
 
@@ skipping 101 matching lines @@
 
     ResetFieldTrialList();
     CreateTestTrial("AutocompleteDynamicTrial_2", "EnableZeroSuggest_Queries");
     EXPECT_TRUE(OmniboxFieldTrial::InZeroSuggestFieldTrial());
 
     ResetFieldTrialList();
     CreateTestTrial("AutocompleteDynamicTrial_3", "EnableZeroSuggest_URLs");
     EXPECT_TRUE(OmniboxFieldTrial::InZeroSuggestFieldTrial());
   }
 }
+
+TEST_F(OmniboxFieldTrialTest, GetValueOfRuleInPageClassificationContext) {
+  // Must be the same as kBundledExperimentFieldTrialName
+  // defined in omnibox_field_trial.cc.
+  const std::string kTrialName = "OmniboxBundledExperimentV1";
+  {
+    std::map<std::string, std::string> params;
+    params["rule1:1"] = "rule1-1-value";

[Peter Kasting, 2013/08/01 22:44:48]

Nit: Might want EOL comments like "// NEW_TAB_PAGE" to clarify what "1"/"3"/"4"
correspond to

[Mark P, 2013/08/02 00:44:36]

Done.

+    params["rule1:3"] = "rule1-3-value";
+    params["rule1:*"] = "rule1-*-value";
+    params["rule2:*"] = "rule2-*-value";
+    params["rule3:4"] = "rule3-4-value";
+    params["unrecognized"] = "unrecognized-value";
+    ASSERT_TRUE(chrome_variations::AssociateVariationParams(
+        kTrialName, "A", params));
+  }
+
+  base::FieldTrialList::CreateFieldTrial(kTrialName, "A");
+  EXPECT_EQ("rule1-1-value",

[Peter Kasting, 2013/08/01 22:44:48]

Tiny nit: All lines of args at the same nesting level should be aligned (so,
you'd probably need to put this alone on a subsequent, indented-4 line).

[Mark P, 2013/08/02 00:44:36]

Done.

+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule1", AutocompleteInput::NEW_TAB_PAGE));  // exact match

[Peter Kasting, 2013/08/01 22:44:48]

Nit: Rather than these EOL comments, how about block comments like:

  // Test exact matches for particular rule/classification pairs.
  EXPECT...
  ...

  // Test mismatches for rules with global "wildcard" fallbacks.
  ...

  // Test mismatches for rules without global fallbacks.
  ...

  // Test nonexistent rules.
  ...

[Mark P, 2013/08/02 00:44:36]

I think this test is more easily understood by looking at each set of cases
about a particular rule.  That's the current order.  Otherwise, for instance, to
examine all tests in each of your particular categories, one would have to keep
all rules in one's head.  This way one only has to keep one set of rules in
one's head at once.

[Peter Kasting, 2013/08/02 00:49:33]

OK, then break the test into blocks that have to deal with each rule, and put
comments like "// Tests for rule 1." or something atop each block.

I'm not as sold as you on this ordering, but at least then it's not an
intimidating wall of verbiage that all looks the same, which is how the function
reads today.

[Mark P, 2013/08/02 19:50:44]

Done.

+  EXPECT_EQ("rule1-*-value",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule1", AutocompleteInput::BLANK));  // fallback to global
+  EXPECT_EQ("rule1-3-value",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule1", AutocompleteInput::HOMEPAGE));  // exact match
+  EXPECT_EQ("rule1-*-value",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule1", AutocompleteInput::OTHER));  // fallback to global
+  EXPECT_EQ("rule2-*-value",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule2", AutocompleteInput::HOMEPAGE));  // fallback to global
+  EXPECT_EQ("rule2-*-value",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule2", AutocompleteInput::OTHER));  // fallback to global
+  EXPECT_EQ("",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule3", AutocompleteInput::BLANK));  // no global
+  EXPECT_EQ("",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule3", AutocompleteInput::HOMEPAGE));  // no global
+  EXPECT_EQ("rule3-4-value",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule3", AutocompleteInput::OTHER));  // exact match
+  EXPECT_EQ("",
+      OmniboxFieldTrial::GetValueOfRuleInPageClassificationContext(
+          "rule4", AutocompleteInput::OTHER));  // no rule at all
+}