Allow trials to associate without overriding a feature.
This adds a heuristic such that if a study only references a single
feature from any of its groups, its other groups will be associated
with that feature as well, for reporting only. This allows a config like
25% Enabled, 25% Disabled and 50% Default to still report the users
in the default group to UMA, so that they show up in the data.
More details in the code comments.
BUG=587135
Review URL: https://codereview.chromium.org/1809633003
Cr-Commit-Position: refs/heads/master@{#381737}
diff --git a/components/variations/variations_seed_processor_unittest.cc b/components/variations/variations_seed_processor_unittest.cc
index d2a29595..693ccf9 100644
--- a/components/variations/variations_seed_processor_unittest.cc
+++ b/components/variations/variations_seed_processor_unittest.cc
@@ -359,6 +359,49 @@
EXPECT_FALSE(processed_study.Init(&study, false));
}
+TEST_F(VariationsSeedProcessorTest, ValidateStudySingleFeature) {
+ Study study;
+ study.set_default_experiment_name("def");
+ Study_Experiment* exp1 = AddExperiment("exp1", 100, &study);
+ Study_Experiment* exp2 = AddExperiment("exp2", 100, &study);
+ Study_Experiment* exp3 = AddExperiment("exp3", 100, &study);
+ AddExperiment("def", 100, &study);
+
+ ProcessedStudy processed_study;
+ EXPECT_TRUE(processed_study.Init(&study, false));
+ EXPECT_EQ(400, processed_study.total_probability());
+
+ EXPECT_EQ(std::string(), processed_study.single_feature_name());
+
+ const char kFeature1Name[] = "Feature1";
+ const char kFeature2Name[] = "Feature2";
+
+ exp1->mutable_feature_association()->add_enable_feature(kFeature1Name);
+ EXPECT_TRUE(processed_study.Init(&study, false));
+ EXPECT_EQ(kFeature1Name, processed_study.single_feature_name());
+
+ exp1->clear_feature_association();
+ exp1->mutable_feature_association()->add_enable_feature(kFeature1Name);
+ exp1->mutable_feature_association()->add_enable_feature(kFeature2Name);
+ EXPECT_TRUE(processed_study.Init(&study, false));
+ // Since there's multiple different features, |single_feature_name| should be
+ // unset.
+ EXPECT_EQ(std::string(), processed_study.single_feature_name());
+
+ exp1->clear_feature_association();
+ exp1->mutable_feature_association()->add_enable_feature(kFeature1Name);
+ exp2->mutable_feature_association()->add_enable_feature(kFeature1Name);
+ exp3->mutable_feature_association()->add_disable_feature(kFeature1Name);
+ EXPECT_TRUE(processed_study.Init(&study, false));
+ EXPECT_EQ(kFeature1Name, processed_study.single_feature_name());
+
+ // Setting a different feature name on exp2 should cause |single_feature_name|
+ // to be not set.
+ exp2->mutable_feature_association()->set_enable_feature(0, kFeature2Name);
+ EXPECT_TRUE(processed_study.Init(&study, false));
+ EXPECT_EQ(std::string(), processed_study.single_feature_name());
+}
+
TEST_F(VariationsSeedProcessorTest, ProcessedStudyAllAssignmentsToOneGroup) {
Study study;
study.set_default_experiment_name("def");
@@ -613,14 +656,13 @@
// Check what happens without and command-line forcing flags - that the
// |one_hundred_percent_group| gets correctly selected and does the right
// thing w.r.t. to affecting the feature / activating the trial.
- {kFeatureOffByDefault, "", "", DEFAULT_GROUP, kDefaultGroup, false,
- false},
+ {kFeatureOffByDefault, "", "", DEFAULT_GROUP, kDefaultGroup, false, true},
{kFeatureOffByDefault, "", "", ENABLE_GROUP, kEnabledGroup, true, true},
{kFeatureOffByDefault, "", "", DISABLE_GROUP, kDisabledGroup, false,
true},
// Do the same as above, but for kFeatureOnByDefault feature.
- {kFeatureOnByDefault, "", "", DEFAULT_GROUP, kDefaultGroup, true, false},
+ {kFeatureOnByDefault, "", "", DEFAULT_GROUP, kDefaultGroup, true, true},
{kFeatureOnByDefault, "", "", ENABLE_GROUP, kEnabledGroup, true, true},
{kFeatureOnByDefault, "", "", DISABLE_GROUP, kDisabledGroup, false, true},
