Make it so disabled field trials are not reported as active.

base/metrics/field_trial_unittest.cc

 // Copyright (c) 2012 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 "base/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 370 matching lines @@
   FieldTrial* trial3 = FieldTrialList::FactoryGetFieldTrial(
       "zzz", 10, "default", next_year_, 12, 31, NULL);
   // Finalize the group selection by accessing the selected group.
   trial3->group();
 
   FieldTrialList::StatesToString(&save_string);
   EXPECT_EQ("Some name/Winner/xxx/yyyy/zzz/default/", save_string);
 }
 
 TEST_F(FieldTrialTest, Restore) {
-  EXPECT_TRUE(FieldTrialList::Find("Some_name") == NULL);
-  EXPECT_TRUE(FieldTrialList::Find("xxx") == NULL);
+  ASSERT_FALSE(FieldTrialList::TrialExists("Some_name"));
+  ASSERT_FALSE(FieldTrialList::TrialExists("xxx"));

[Ilya Sherman, 2012/11/09 22:45:59]

Just curious, why did you change these from EXPECTs to ASSERTs?

[Alexei Svitkine (slow), 2012/11/09 23:42:39]

It's a precondition, if that line fails, then the rest of the test results
aren't valid, so the test might as well not bother running the rest and exit
early.

 
   FieldTrialList::CreateTrialsFromString("Some_name/Winner/xxx/yyyy/");
 
   FieldTrial* trial = FieldTrialList::Find("Some_name");
   ASSERT_NE(static_cast<FieldTrial*>(NULL), trial);
   EXPECT_EQ("Winner", trial->group_name());
   EXPECT_EQ("Some_name", trial->name());
 
   trial = FieldTrialList::Find("xxx");
   ASSERT_NE(static_cast<FieldTrial*>(NULL), trial);
@@ skipping 21 matching lines @@
   EXPECT_EQ("Some name/Winner/", save_string);
 
   // It is OK if we redundantly specify a winner.
   EXPECT_TRUE(FieldTrialList::CreateTrialsFromString(save_string));
 
   // But it is an error to try to change to a different winner.
   EXPECT_FALSE(FieldTrialList::CreateTrialsFromString("Some name/Loser/"));
 }
 
 TEST_F(FieldTrialTest, CreateFieldTrial) {
-  EXPECT_TRUE(FieldTrialList::Find("Some_name") == NULL);
+  ASSERT_FALSE(FieldTrialList::TrialExists("Some_name"));
 
   FieldTrialList::CreateFieldTrial("Some_name", "Winner");
 
   FieldTrial* trial = FieldTrialList::Find("Some_name");
   ASSERT_NE(static_cast<FieldTrial*>(NULL), trial);
   EXPECT_EQ("Winner", trial->group_name());
   EXPECT_EQ("Some_name", trial->name());
 }
 
 TEST_F(FieldTrialTest, DuplicateFieldTrial) {
@@ skipping 125 matching lines @@
   if (chosen_group == default_group)
     EXPECT_EQ(kDefaultGroupName, observer.group_name());
   else
     EXPECT_EQ(kSecondaryGroupName, observer.group_name());
 }
 
 TEST_F(FieldTrialTest, ObserveDisabled) {
   const char kTrialName[] = "TrialToObserve2";
 
   TestFieldTrialObserver observer;
+  int default_group = -1;
   FieldTrial* trial =
       FieldTrialList::FactoryGetFieldTrial(kTrialName, 100, kDefaultGroupName,
-                                           next_year_, 12, 31, NULL);
+                                           next_year_, 12, 31, &default_group);
   trial->AppendGroup("A", 25);
   trial->AppendGroup("B", 25);
   trial->AppendGroup("C", 25);
   trial->Disable();
 
-  // Observer shouldn't be notified until group() is called.
+  // Observer shouldn't be notified of a disabled trial.
   message_loop_.RunAllPending();
   EXPECT_TRUE(observer.trial_name().empty());
   EXPECT_TRUE(observer.group_name().empty());
 
-  trial->group();
+  // Observer shouldn't be notified even after a |group()| call.
+  EXPECT_EQ(default_group, trial->group());
   message_loop_.RunAllPending();
-  EXPECT_EQ(kTrialName, observer.trial_name());
-  EXPECT_EQ(kDefaultGroupName, observer.group_name());
+  EXPECT_TRUE(observer.trial_name().empty());
+  EXPECT_TRUE(observer.group_name().empty());
 }
 
 TEST_F(FieldTrialTest, ObserveForcedDisabled) {
   const char kTrialName[] = "TrialToObserve3";
 
   TestFieldTrialObserver observer;
+  int default_group = -1;
   FieldTrial* trial =
       FieldTrialList::FactoryGetFieldTrial(kTrialName, 100, kDefaultGroupName,
-                                           next_year_, 12, 31, NULL);
+                                           next_year_, 12, 31, &default_group);
   trial->AppendGroup("A", 25);
   trial->AppendGroup("B", 25);
   trial->AppendGroup("C", 25);
   trial->SetForced();
   trial->Disable();
 
-  // Observer shouldn't be notified until group() is called, even if SetForced()
-  // was called.
+  // Observer shouldn't be notified of a disabled trial, even when forced.
   message_loop_.RunAllPending();
   EXPECT_TRUE(observer.trial_name().empty());
   EXPECT_TRUE(observer.group_name().empty());
 
-  trial->group();
+  // Observer shouldn't be notified even after a |group()| call.
+  EXPECT_EQ(default_group, trial->group());
   message_loop_.RunAllPending();
-  EXPECT_EQ(kTrialName, observer.trial_name());
-  EXPECT_EQ(kDefaultGroupName, observer.group_name());
+  EXPECT_TRUE(observer.trial_name().empty());
+  EXPECT_TRUE(observer.group_name().empty());
 }
 
+TEST_F(FieldTrialTest, DisabledTrialNotActive) {
+  const char kTrialName[] = "DisabledTrial";
+  ASSERT_FALSE(FieldTrialList::TrialExists(kTrialName));
+
+  FieldTrial* trial =
+      FieldTrialList::FactoryGetFieldTrial(kTrialName, 100, kDefaultGroupName,
+                                           next_year_, 12, 31, NULL);
+  trial->AppendGroup("X", 50);
+  trial->Disable();
+
+  // Ensure the trial is not listed as active.
+  FieldTrial::ActiveGroups active_groups;
+  FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
+  EXPECT_TRUE(active_groups.empty());
+
+  // Ensure the trial is not listed in the |StatesToString()| result.
+  std::string states;
+  FieldTrialList::StatesToString(&states);
+  EXPECT_TRUE(states.empty());
+}
+
+
 }  // namespace base