Commit 8092917f authored by Alexei Svitkine's avatar Alexei Svitkine Committed by Commit Bot

Remove special handling of empty variations platforms list.

Previously, it would be treated as matching all platforms. This
makes it match none, which matches what server behavior is for
serving configs given a platform request param.

Bug: None
Change-Id: I2241c664c5b8dd12c689a84aec8fd779c78b81c5
Reviewed-on: https://chromium-review.googlesource.com/c/1473614
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632636}
parent e40580f9
......@@ -30,7 +30,6 @@
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_ANDROID)
#include "components/variations/android/variations_seed_bridge.h"
#include "components/variations/seed_response.h"
#endif // OS_ANDROID
......@@ -99,6 +98,7 @@ VariationsSeed CreateTestSeedWithCountryFilter() {
Study* study = seed.mutable_study(0);
Study::Filter* filter = study->mutable_filter();
filter->add_country(kTestSeedCountry);
filter->add_platform(Study::PLATFORM_ANDROID);
return seed;
}
......
......@@ -107,10 +107,6 @@ bool CheckStudyLocale(const Study::Filter& filter, const std::string& locale) {
}
bool CheckStudyPlatform(const Study::Filter& filter, Study::Platform platform) {
// An empty platform list matches all platforms.
if (filter.platform_size() == 0)
return true;
for (int i = 0; i < filter.platform_size(); ++i) {
if (filter.platform(i) == platform)
return true;
......
......@@ -222,10 +222,10 @@ TEST(VariationsStudyFilteringTest, CheckStudyPlatform) {
Study::Filter filter;
// Check in the forwarded order. The loop cond is <= base::size(platforms)
// instead of < so that the result of adding the last channel gets checked.
// instead of < so that the result of adding the last platform gets checked.
for (size_t i = 0; i <= base::size(platforms); ++i) {
for (size_t j = 0; j < base::size(platforms); ++j) {
const bool expected = platform_added[j] || filter.platform_size() == 0;
const bool expected = platform_added[j];
const bool result = internal::CheckStudyPlatform(filter, platforms[j]);
EXPECT_EQ(expected, result) << "Case " << i << "," << j << " failed!";
}
......@@ -241,7 +241,7 @@ TEST(VariationsStudyFilteringTest, CheckStudyPlatform) {
memset(&platform_added, 0, sizeof(platform_added));
for (size_t i = 0; i <= base::size(platforms); ++i) {
for (size_t j = 0; j < base::size(platforms); ++j) {
const bool expected = platform_added[j] || filter.platform_size() == 0;
const bool expected = platform_added[j];
const bool result = internal::CheckStudyPlatform(filter, platforms[j]);
EXPECT_EQ(expected, result) << "Case " << i << "," << j << " failed!";
}
......@@ -575,6 +575,7 @@ TEST(VariationsStudyFilteringTest, FilterAndValidateStudiesWithCountry) {
study->set_default_experiment_name("Default");
AddExperiment("Default", 100, study);
study->set_consistency(test.consistency);
study->mutable_filter()->add_platform(Study::PLATFORM_ANDROID);
if (test.filter_country)
study->mutable_filter()->add_country(test.filter_country);
if (test.filter_exclude_country)
......@@ -585,7 +586,7 @@ TEST(VariationsStudyFilteringTest, FilterAndValidateStudiesWithCountry) {
client_state.reference_date = base::Time::Now();
client_state.version = base::Version("20.0.0.0");
client_state.channel = Study::STABLE;
client_state.form_factor = Study::DESKTOP;
client_state.form_factor = Study::PHONE;
client_state.platform = Study::PLATFORM_ANDROID;
client_state.session_consistency_country = kSessionCountry;
client_state.permanent_consistency_country = kPermanentCountry;
......
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