Skip to content

GitLab

T
tangled
Commit ce49a7d0 authored by Wez's avatar Wez Committed by Commit Bot
Browse files
Options

[base] Clean up FeatureListTest to use base::test::ScopedFeatureList. 

Migrate the FeatureListTest suite to use ScopedFeatureList to ensure
that the process-global FeatureList instance is replaced & restored
consistently.

Change-Id: Ia9dd5bf7269ab05d00780c25c8f3154d663546e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848354Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703929}
parent ba05a830
Hide whitespace changes
Inline Side-by-side
Showing with 64 additions and 72 deletions
base/feature_list_unittest.cc
View file @ ce49a7d0
......@@ -18,6 +18,7 @@
#include "base/strings/string_piece.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/test/scoped_feature_list.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace base {
......@@ -45,26 +46,14 @@ std::string SortFeatureListString(const std::string& feature_list) {
class FeatureListTest : public testing::Test {
public:
FeatureListTest() : feature_list_(nullptr) {
RegisterFeatureListInstance(std::make_unique<FeatureList>());
FeatureListTest() {
// Provide an empty FeatureList to each test by default.
scoped_feature_list_.InitWithFeatureList(std::make_unique<FeatureList>());
}
~FeatureListTest() override { ClearFeatureListInstance(); }
void RegisterFeatureListInstance(std::unique_ptr<FeatureList> feature_list) {
FeatureList::ClearInstanceForTesting();
feature_list_ = feature_list.get();
FeatureList::SetInstance(std::move(feature_list));
}
void ClearFeatureListInstance() {
FeatureList::ClearInstanceForTesting();
feature_list_ = nullptr;
}
FeatureList* feature_list() { return feature_list_; }
~FeatureListTest() override = default;
private:
// Weak. Owned by the FeatureList::SetInstance().
FeatureList* feature_list_;
test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(FeatureListTest);
};
......@@ -96,11 +85,11 @@ TEST_F(FeatureListTest, InitializeFromCommandLine) {
test_case.enable_features,
test_case.disable_features));
ClearFeatureListInstance();
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine(test_case.enable_features,
test_case.disable_features);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_EQ(test_case.expected_feature_on_state,
FeatureList::IsEnabled(kFeatureOnByDefault))
......@@ -115,19 +104,23 @@ TEST_F(FeatureListTest, CheckFeatureIdentity) {
// Tests that CheckFeatureIdentity() correctly detects when two different
// structs with the same feature name are passed to it.
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::make_unique<FeatureList>());
FeatureList* feature_list = FeatureList::GetInstance();
// Call it twice for each feature at the top of the file, since the first call
// makes it remember the entry and the second call will verify it.
EXPECT_TRUE(feature_list()->CheckFeatureIdentity(kFeatureOnByDefault));
EXPECT_TRUE(feature_list()->CheckFeatureIdentity(kFeatureOnByDefault));
EXPECT_TRUE(feature_list()->CheckFeatureIdentity(kFeatureOffByDefault));
EXPECT_TRUE(feature_list()->CheckFeatureIdentity(kFeatureOffByDefault));
EXPECT_TRUE(feature_list->CheckFeatureIdentity(kFeatureOnByDefault));
EXPECT_TRUE(feature_list->CheckFeatureIdentity(kFeatureOnByDefault));
EXPECT_TRUE(feature_list->CheckFeatureIdentity(kFeatureOffByDefault));
EXPECT_TRUE(feature_list->CheckFeatureIdentity(kFeatureOffByDefault));
// Now, call it with a distinct struct for |kFeatureOnByDefaultName|, which
// should return false.
struct Feature kFeatureOnByDefault2 {
kFeatureOnByDefaultName, FEATURE_ENABLED_BY_DEFAULT
};
EXPECT_FALSE(feature_list()->CheckFeatureIdentity(kFeatureOnByDefault2));
EXPECT_FALSE(feature_list->CheckFeatureIdentity(kFeatureOnByDefault2));
}
TEST_F(FeatureListTest, FieldTrialOverrides) {
......@@ -150,10 +143,8 @@ TEST_F(FeatureListTest, FieldTrialOverrides) {
const auto& test_case = test_cases[i];
SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]", i));
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
FieldTrial* trial1 = FieldTrialList::CreateFieldTrial("TrialExample1", "A");
FieldTrial* trial2 = FieldTrialList::CreateFieldTrial("TrialExample2", "B");
......@@ -161,7 +152,8 @@ TEST_F(FeatureListTest, FieldTrialOverrides) {
test_case.trial1_state, trial1);
feature_list->RegisterFieldTrialOverride(kFeatureOffByDefaultName,
test_case.trial2_state, trial2);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
// Initially, neither trial should be active.
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial1->trial_name()));
......@@ -185,7 +177,7 @@ TEST_F(FeatureListTest, FieldTrialOverrides) {
TEST_F(FeatureListTest, FieldTrialAssociateUseDefault) {
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
FieldTrial* trial1 = FieldTrialList::CreateFieldTrial("TrialExample1", "A");
FieldTrial* trial2 = FieldTrialList::CreateFieldTrial("TrialExample2", "B");
......@@ -193,7 +185,8 @@ TEST_F(FeatureListTest, FieldTrialAssociateUseDefault) {
kFeatureOnByDefaultName, FeatureList::OVERRIDE_USE_DEFAULT, trial1);
feature_list->RegisterFieldTrialOverride(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_USE_DEFAULT, trial2);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
// Initially, neither trial should be active.
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial1->trial_name()));
......@@ -213,10 +206,8 @@ TEST_F(FeatureListTest, FieldTrialAssociateUseDefault) {
}
TEST_F(FeatureListTest, CommandLineEnableTakesPrecedenceOverFieldTrial) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
// The feature is explicitly enabled on the command-line.
feature_list->InitializeFromCommandLine(kFeatureOffByDefaultName, "");
......@@ -225,7 +216,8 @@ TEST_F(FeatureListTest, CommandLineEnableTakesPrecedenceOverFieldTrial) {
FieldTrial* trial = FieldTrialList::CreateFieldTrial("TrialExample2", "A");
feature_list->RegisterFieldTrialOverride(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE, trial);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial->trial_name()));
// Command-line should take precedence.
......@@ -237,10 +229,8 @@ TEST_F(FeatureListTest, CommandLineEnableTakesPrecedenceOverFieldTrial) {
}
TEST_F(FeatureListTest, CommandLineDisableTakesPrecedenceOverFieldTrial) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
// The feature is explicitly disabled on the command-line.
feature_list->InitializeFromCommandLine("", kFeatureOffByDefaultName);
......@@ -249,7 +239,8 @@ TEST_F(FeatureListTest, CommandLineDisableTakesPrecedenceOverFieldTrial) {
FieldTrial* trial = FieldTrialList::CreateFieldTrial("TrialExample2", "A");
feature_list->RegisterFieldTrialOverride(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial->trial_name()));
// Command-line should take precedence.
......@@ -261,10 +252,8 @@ TEST_F(FeatureListTest, CommandLineDisableTakesPrecedenceOverFieldTrial) {
}
TEST_F(FeatureListTest, IsFeatureOverriddenFromCommandLine) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
// No features are overridden from the command line yet
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
......@@ -304,7 +293,8 @@ TEST_F(FeatureListTest, IsFeatureOverriddenFromCommandLine) {
kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
// Check the expected feature states for good measure.
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOffByDefault));
......@@ -336,10 +326,8 @@ TEST_F(FeatureListTest, AssociateReportingFieldTrial) {
test_case.enable_features,
test_case.disable_features));
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine(test_case.enable_features,
test_case.disable_features);
......@@ -364,7 +352,8 @@ TEST_F(FeatureListTest, AssociateReportingFieldTrial) {
EXPECT_EQ(test_case.expected_enable_trial_created, enable_trial != nullptr);
EXPECT_EQ(test_case.expected_disable_trial_created,
disable_trial != nullptr);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_FALSE(FieldTrialList::IsTrialActive(kTrialName));
if (disable_trial) {
......@@ -380,8 +369,6 @@ TEST_F(FeatureListTest, AssociateReportingFieldTrial) {
}
TEST_F(FeatureListTest, RegisterExtraFeatureOverrides) {
ClearFeatureListInstance();
auto feature_list = std::make_unique<FeatureList>();
std::vector<FeatureList::FeatureOverrideInfo> overrides;
overrides.push_back({std::cref(kFeatureOnByDefault),
......@@ -389,15 +376,14 @@ TEST_F(FeatureListTest, RegisterExtraFeatureOverrides) {
overrides.push_back({std::cref(kFeatureOffByDefault),
FeatureList::OverrideState::OVERRIDE_ENABLE_FEATURE});
feature_list->RegisterExtraFeatureOverrides(std::move(overrides));
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOnByDefault));
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOffByDefault));
}
TEST_F(FeatureListTest, InitializeFromCommandLineThenRegisterExtraOverrides) {
ClearFeatureListInstance();
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine(kFeatureOnByDefaultName,
kFeatureOffByDefaultName);
......@@ -407,7 +393,8 @@ TEST_F(FeatureListTest, InitializeFromCommandLineThenRegisterExtraOverrides) {
overrides.push_back({std::cref(kFeatureOffByDefault),
FeatureList::OverrideState::OVERRIDE_ENABLE_FEATURE});
feature_list->RegisterExtraFeatureOverrides(std::move(overrides));
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
// The InitializeFromCommandLine supersedes the RegisterExtraFeatureOverrides
// because it was called first.
......@@ -423,9 +410,8 @@ TEST_F(FeatureListTest, InitializeFromCommandLineThenRegisterExtraOverrides) {
}
TEST_F(FeatureListTest, GetFeatureOverrides) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine("A,X", "D");
Feature feature_b = {"B", FEATURE_ENABLED_BY_DEFAULT};
......@@ -442,7 +428,8 @@ TEST_F(FeatureListTest, GetFeatureOverrides) {
FeatureList::OVERRIDE_ENABLE_FEATURE,
trial);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
std::string enable_features;
std::string disable_features;
......@@ -458,16 +445,16 @@ TEST_F(FeatureListTest, GetFeatureOverrides) {
}
TEST_F(FeatureListTest, GetFeatureOverrides_UseDefault) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine("A,X", "D");
FieldTrial* trial = FieldTrialList::CreateFieldTrial("Trial", "Group");
feature_list->RegisterFieldTrialOverride(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_USE_DEFAULT, trial);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
std::string enable_features;
std::string disable_features;
......@@ -478,25 +465,25 @@ TEST_F(FeatureListTest, GetFeatureOverrides_UseDefault) {
}
TEST_F(FeatureListTest, GetFieldTrial) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
FieldTrial* trial = FieldTrialList::CreateFieldTrial("Trial", "Group");
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->RegisterFieldTrialOverride(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_USE_DEFAULT, trial);
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_EQ(trial, FeatureList::GetFieldTrial(kFeatureOnByDefault));
EXPECT_EQ(nullptr, FeatureList::GetFieldTrial(kFeatureOffByDefault));
}
TEST_F(FeatureListTest, InitializeFromCommandLine_WithFieldTrials) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
FieldTrialList::CreateFieldTrial("Trial", "Group");
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine("A,OffByDefault<Trial,X", "D");
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_FALSE(FieldTrialList::IsTrialActive("Trial"));
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOffByDefault));
......@@ -504,14 +491,14 @@ TEST_F(FeatureListTest, InitializeFromCommandLine_WithFieldTrials) {
}
TEST_F(FeatureListTest, InitializeFromCommandLine_UseDefault) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
FieldTrialList::CreateFieldTrial("T1", "Group");
FieldTrialList::CreateFieldTrial("T2", "Group");
std::unique_ptr<FeatureList> feature_list(new FeatureList);
auto feature_list = std::make_unique<FeatureList>();
feature_list->InitializeFromCommandLine(
"A,*OffByDefault<T1,*OnByDefault<T2,X", "D");
RegisterFeatureListInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_FALSE(FieldTrialList::IsTrialActive("T1"));
EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOffByDefault));
......@@ -523,10 +510,10 @@ TEST_F(FeatureListTest, InitializeFromCommandLine_UseDefault) {
}
TEST_F(FeatureListTest, InitializeInstance) {
ClearFeatureListInstance();
std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
FeatureList::SetInstance(std::move(feature_list));
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatureList(std::move(feature_list));
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOnByDefault));
EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOffByDefault));
......@@ -542,7 +529,9 @@ TEST_F(FeatureListTest, InitializeInstance) {
}
TEST_F(FeatureListTest, UninitializedInstance_IsEnabledReturnsFalse) {
ClearFeatureListInstance();
std::unique_ptr<FeatureList> original_feature_list =
FeatureList::ClearInstanceForTesting();
// This test case simulates the calling pattern found in code which does not
// explicitly initialize the features list.
// All IsEnabled() calls should return the default value in this scenario.
......@@ -550,6 +539,9 @@ TEST_F(FeatureListTest, UninitializedInstance_IsEnabledReturnsFalse) {
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOnByDefault));
EXPECT_EQ(nullptr, FeatureList::GetInstance());
EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOffByDefault));
if (original_feature_list)
FeatureList::RestoreInstanceForTesting(std::move(original_feature_list));
}
TEST_F(FeatureListTest, StoreAndRetrieveFeaturesFromSharedMemory) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment