variations::testing::VariationParamsManager does not set values after clearing inital values |
||
Issue description
The method |SetVariationParamsWithFeatureAssociations| does not set parameters after calling |ClearAllVariationParams|.
(1) Call |SetVariationParamsWithFeatureAssociations| with params {"blubb", "true"}. "blubb" is set.
(2) Call |ClearAllVariation{Param,ID}s| to delete all variation. --> "blubb" is unset.
(3) Call |SetVariationParamsWithFeatureAssociations| with params {"blubb", "true"}. "blubb" is still unset.
What is the expected result?
The values are set to the requested values.
What happens instead?
Nothing. It doesn't crash or show any errors, the values just stay undefined.
How you can work around this issue:
Destroy the instance of the VariationParamsManager completely and create a new instance.
How |SetFetchingPersonalization| in /components/ntp_snippets/remote/ntp_snippets_fetcher_unittest.cc [1] should work: (expectations optional)
void SetFetchingPersonalizationVariation(
const std::string& personalization_string) {
EXPECT_THAT(variations::GetVariationParamValue(ntp_snippets::kStudyName,
"send_top_languages"),
Eq("true"));
EXPECT_THAT(variations::GetVariationParamValue(ntp_snippets::kStudyName,
"fetching_personalization"),
Eq(""));
params_manager_.ClearAllVariationIDs();
params_manager_.ClearAllVariationParams();
std::map<std::string, std::string> params = GetDefaultVariationParameters();
params["fetching_personalization"] = personalization_string;
params_manager_.SetVariationParamsWithFeatureAssociations(
ntp_snippets::kStudyName, params,
{ntp_snippets::kArticleSuggestionsFeature.name});
EXPECT_THAT(variations::GetVariationParamValue(ntp_snippets::kStudyName,
"send_top_languages"),
Eq("true"));
EXPECT_THAT(variations::GetVariationParamValue(ntp_snippets::kStudyName,
"fetching_personalization"),
Eq("personalization_string"));
}
[1] This file might be named /components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.cc by now.
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e0bf6128f18eedd05c1525c09d258df0080e8583 commit e0bf6128f18eedd05c1525c09d258df0080e8583 Author: fhorschig <fhorschig@chromium.org> Date: Tue Dec 20 16:53:06 2016 Make FieldTrialList usable after clearing VariationsParamsManager. Calling |ClearAllVariationParams| will now create a new instance of the FieldTrialList so |SetVariationParamsWithFeatureAssociations| will work again. BUG= 672010 Review-Url: https://codereview.chromium.org/2590883002 Cr-Commit-Position: refs/heads/master@{#439824} [modify] https://crrev.com/e0bf6128f18eedd05c1525c09d258df0080e8583/components/variations/variations_params_manager.cc
,
Dec 20 2016
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08db2becc667bf907330d414f372f4c297b8a54f commit 08db2becc667bf907330d414f372f4c297b8a54f Author: fhorschig <fhorschig@chromium.org> Date: Tue Dec 20 17:46:42 2016 NTP: VariationParamsManager resetting works, so make it a member. This CL makes use of the resetting function VariationParamsManager. This should work now and therefore it is sufficient to hold the manager as a simple member. BUG= 672010 Review-Url: https://codereview.chromium.org/2592663003 Cr-Commit-Position: refs/heads/master@{#439839} [modify] https://crrev.com/08db2becc667bf907330d414f372f4c297b8a54f/components/ntp_snippets/remote/ntp_snippets_fetcher_unittest.cc |
||
►
Sign in to add a comment |
||
Comment 1 by fhorschig@chromium.org
, Dec 20 2016