Skip to content

Commit

Permalink
Do not instantiate base::FieldTrialList explicitly in tests
Browse files Browse the repository at this point in the history
The test suite already instantiates a FieldTrialList as per
https://chromium-review.googlesource.com/c/chromium/src/+/1883567 so
it's no longer necessary to do so explicitly in tests.

This patch addresses unit tests under:
/components/omnibox

This CL was uploaded by git cl split.

R=mpearson@chromium.org

Bug: 1018667
Change-Id: Idb0b248873001282c98db87a6dfc6381d45bbbc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1917229
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716144}
  • Loading branch information
Mikel Astiz authored and Commit Bot committed Nov 18, 2019
1 parent 75ba020 commit 3997443
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 27 deletions.
7 changes: 0 additions & 7 deletions components/omnibox/browser/autocomplete_result_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "components/omnibox/browser/test_scheme_classifier.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/search_engines/template_url_service.h"
#include "components/variations/entropy_provider.h"
#include "components/variations/variations_associated_data.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -113,11 +112,6 @@ class AutocompleteResultTest : public testing::Test {
};

AutocompleteResultTest() {
// Destroy the existing FieldTrialList before creating a new one to avoid
// a DCHECK.
field_trial_list_.reset();
field_trial_list_.reset(new base::FieldTrialList(
std::make_unique<variations::SHA1EntropyProvider>("foo")));
variations::testing::ClearAllVariationParams();

// Create the list of mock providers. 5 is enough.
Expand Down Expand Up @@ -168,7 +162,6 @@ class AutocompleteResultTest : public testing::Test {

private:
base::test::TaskEnvironment task_environment_;
std::unique_ptr<base::FieldTrialList> field_trial_list_;

// For every provider mentioned in TestData, we need a mock provider.
std::vector<scoped_refptr<MockAutocompleteProvider> > mock_provider_list_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class DocumentSuggestionsServiceTest : public testing::Test {
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)),
identity_test_env_(&test_url_loader_factory_, &prefs_),
field_trial_list_(nullptr),
document_suggestions_service_(new DocumentSuggestionsService(
identity_test_env_.identity_manager(),
shared_url_loader_factory_)) {
Expand All @@ -64,7 +63,6 @@ class DocumentSuggestionsServiceTest : public testing::Test {
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
sync_preferences::TestingPrefServiceSyncable prefs_;
signin::IdentityTestEnvironment identity_test_env_;
base::FieldTrialList field_trial_list_;
std::unique_ptr<DocumentSuggestionsService> document_suggestions_service_;

DISALLOW_COPY_AND_ASSIGN(DocumentSuggestionsServiceTest);
Expand Down
7 changes: 0 additions & 7 deletions components/omnibox/browser/keyword_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "components/search_engines/search_engines_switches.h"
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_service.h"
#include "components/variations/entropy_provider.h"
#include "components/variations/variations_associated_data.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -64,11 +63,6 @@ class KeywordProviderTest : public testing::Test {
};

KeywordProviderTest() : kw_provider_(nullptr) {
// Destroy the existing FieldTrialList before creating a new one to avoid
// a DCHECK.
field_trial_list_.reset();
field_trial_list_.reset(new base::FieldTrialList(
std::make_unique<variations::SHA1EntropyProvider>("foo")));
variations::testing::ClearAllVariationParams();
}
~KeywordProviderTest() override {}
Expand All @@ -90,7 +84,6 @@ class KeywordProviderTest : public testing::Test {
static const TemplateURLService::Initializer kTestData[];

base::test::TaskEnvironment task_environment_;
std::unique_ptr<base::FieldTrialList> field_trial_list_;
scoped_refptr<KeywordProvider> kw_provider_;
std::unique_ptr<MockAutocompleteProviderClient> client_;
};
Expand Down
26 changes: 15 additions & 11 deletions components/omnibox/browser/omnibox_field_trial_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "build/build_config.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/search/search.h"
#include "components/variations/entropy_provider.h"
#include "components/variations/variations_associated_data.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/metrics_proto/omnibox_event.pb.h"
Expand All @@ -31,12 +30,18 @@ class OmniboxFieldTrialTest : public testing::Test {
}

void ResetFieldTrialList() {
// Destroy the existing FieldTrialList before creating a new one to avoid
// a DCHECK.
field_trial_list_.reset();
field_trial_list_.reset(new base::FieldTrialList(
std::make_unique<variations::SHA1EntropyProvider>("foo")));
scoped_feature_list_.Reset();
variations::testing::ClearAllVariationParams();
scoped_feature_list_.Init();
}

void ResetAndEnableFeatureWithParameters(
const base::Feature& feature,
const base::FieldTrialParams& feature_parameters) {
scoped_feature_list_.Reset();
variations::testing::ClearAllVariationParams();
scoped_feature_list_.InitAndEnableFeatureWithParameters(feature,
feature_parameters);
}

// Creates and activates a field trial.
Expand Down Expand Up @@ -73,7 +78,7 @@ class OmniboxFieldTrialTest : public testing::Test {
int expected_delay_ms);

private:
std::unique_ptr<base::FieldTrialList> field_trial_list_;
base::test::ScopedFeatureList scoped_feature_list_;

DISALLOW_COPY_AND_ASSIGN(OmniboxFieldTrialTest);
};
Expand Down Expand Up @@ -210,8 +215,7 @@ TEST_F(OmniboxFieldTrialTest, GetDemotionsByTypeWithFallback) {

TEST_F(OmniboxFieldTrialTest, GetProviderMaxMatches) {
{
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ResetAndEnableFeatureWithParameters(
omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesByProviderParam,
"1:50,2:0"}});
Expand All @@ -223,8 +227,7 @@ TEST_F(OmniboxFieldTrialTest, GetProviderMaxMatches) {
AutocompleteProvider::Type::TYPE_HISTORY_QUICK));
}
{
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ResetAndEnableFeatureWithParameters(
omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesByProviderParam,
"1:60,*:61,2:62"}});
Expand All @@ -236,6 +239,7 @@ TEST_F(OmniboxFieldTrialTest, GetProviderMaxMatches) {
AutocompleteProvider::Type::TYPE_HISTORY_QUICK));
}
{
ResetFieldTrialList();
ASSERT_EQ(3ul, OmniboxFieldTrial::GetProviderMaxMatches(
AutocompleteProvider::Type::TYPE_BOOKMARK));
ASSERT_EQ(3ul, OmniboxFieldTrial::GetProviderMaxMatches(
Expand Down

0 comments on commit 3997443

Please sign in to comment.