Revert "Removes string support in field trial parser."

This reverts commit e74156f7d05cf3c9858e554789b3f4bb3b93cc19.

Reason for revert: This turned out to be useful :)

Original change's description:
> Removes string support in field trial parser.
> 
> This prepares for simplifying the behavior of optionals so that
> an empty parameter value resets the optional.
> 
> Bug: webrtc:9883
> Change-Id: I8ef8fe9698235044cac66bc4a587abe874c8f854
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150883
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Reviewed-by: Björn Terelius <terelius@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29061}

TBR=terelius@webrtc.org,srte@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:9883
Change-Id: Idbb4061f4b423987e62f3a9ad9bee2410e2cec96
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/152383
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29137}
This commit is contained in:
Philip Eliasson
2019-09-10 14:05:43 +00:00
committed by Commit Bot
parent 507f43465b
commit 29ab487ea7
4 changed files with 65 additions and 31 deletions

View File

@ -20,14 +20,14 @@ namespace webrtc {
struct Garment {
int price = 0;
TimeDelta age = TimeDelta::Zero();
std::string color = "";
// Only needed for testing.
Garment() = default;
Garment(int p, TimeDelta a) : price(p), age(a) {}
Garment(int p, std::string c) : price(p), color(c) {}
bool operator==(const Garment& other) const {
return price == other.price && age == other.age;
return price == other.price && color == other.color;
}
};
@ -43,23 +43,34 @@ TEST(FieldTrialListTest, ParsesListParameter) {
EXPECT_THAT(my_list.Get(), ElementsAre(1, 2, 3));
ParseFieldTrial({&my_list}, "l:-1");
EXPECT_THAT(my_list.Get(), ElementsAre(-1));
FieldTrialList<std::string> another_list("l", {"hat"});
EXPECT_THAT(another_list.Get(), ElementsAre("hat"));
ParseFieldTrial({&another_list}, "l");
EXPECT_THAT(another_list.Get(), IsEmpty());
ParseFieldTrial({&another_list}, "l:");
EXPECT_THAT(another_list.Get(), ElementsAre(""));
ParseFieldTrial({&another_list}, "l:scarf|hat|mittens");
EXPECT_THAT(another_list.Get(), ElementsAre("scarf", "hat", "mittens"));
ParseFieldTrial({&another_list}, "l:scarf");
EXPECT_THAT(another_list.Get(), ElementsAre("scarf"));
}
// Normal usage.
TEST(FieldTrialListTest, ParsesStructList) {
FieldTrialStructList<Garment> my_list(
{FieldTrialStructMember("age", [](Garment* g) { return &g->age; }),
{FieldTrialStructMember("color", [](Garment* g) { return &g->color; }),
FieldTrialStructMember("price", [](Garment* g) { return &g->price; })},
{{1, TimeDelta::seconds(100)}, {2, TimeDelta::PlusInfinity()}});
{{1, "blue"}, {2, "red"}});
ParseFieldTrial({&my_list},
"age:inf|10s|80ms,"
"color:mauve|red|gold,"
"price:10|20|30,"
"other_param:asdf");
ASSERT_THAT(my_list.Get(), ElementsAre(Garment{10, TimeDelta::PlusInfinity()},
Garment{20, TimeDelta::seconds(10)},
Garment{30, TimeDelta::ms(80)}));
ASSERT_THAT(my_list.Get(),
ElementsAre(Garment{10, "mauve"}, Garment{20, "red"},
Garment{30, "gold"}));
}
// One FieldTrialList has the wrong length, so we use the user-provided default
@ -67,57 +78,54 @@ TEST(FieldTrialListTest, ParsesStructList) {
TEST(FieldTrialListTest, StructListKeepsDefaultWithMismatchingLength) {
FieldTrialStructList<Garment> my_list(
{FieldTrialStructMember("wrong_length",
[](Garment* g) { return &g->age; }),
[](Garment* g) { return &g->color; }),
FieldTrialStructMember("price", [](Garment* g) { return &g->price; })},
{{1, TimeDelta::seconds(100)}, {2, TimeDelta::PlusInfinity()}});
{{1, "blue"}, {2, "red"}});
ParseFieldTrial({&my_list},
"wrong_length:3|2|4|3,"
"wrong_length:mauve|magenta|chartreuse|indigo,"
"garment:hat|hat|crown,"
"price:10|20|30");
ASSERT_THAT(my_list.Get(),
ElementsAre(Garment{1, TimeDelta::seconds(100)},
Garment{2, TimeDelta::PlusInfinity()}));
ElementsAre(Garment{1, "blue"}, Garment{2, "red"}));
}
// One list is missing. We set the values we're given, and the others remain
// as whatever the Garment default constructor set them to.
TEST(FieldTrialListTest, StructListUsesDefaultForMissingList) {
FieldTrialStructList<Garment> my_list(
{FieldTrialStructMember("age", [](Garment* g) { return &g->age; }),
{FieldTrialStructMember("color", [](Garment* g) { return &g->color; }),
FieldTrialStructMember("price", [](Garment* g) { return &g->price; })},
{{1, TimeDelta::seconds(100)}, {2, TimeDelta::PlusInfinity()}});
{{1, "blue"}, {2, "red"}});
ParseFieldTrial({&my_list}, "price:10|20|30");
ASSERT_THAT(my_list.Get(), ElementsAre(Garment{10, TimeDelta::Zero()},
Garment{20, TimeDelta::Zero()},
Garment{30, TimeDelta::Zero()}));
ASSERT_THAT(my_list.Get(),
ElementsAre(Garment{10, ""}, Garment{20, ""}, Garment{30, ""}));
}
// The user haven't provided values for any lists, so we use the default list.
TEST(FieldTrialListTest, StructListUsesDefaultListWithoutValues) {
FieldTrialStructList<Garment> my_list(
{FieldTrialStructMember("age", [](Garment* g) { return &g->age; }),
{FieldTrialStructMember("color", [](Garment* g) { return &g->color; }),
FieldTrialStructMember("price", [](Garment* g) { return &g->price; })},
{{1, TimeDelta::seconds(100)}, {2, TimeDelta::PlusInfinity()}});
{{1, "blue"}, {2, "red"}});
ParseFieldTrial({&my_list}, "");
ASSERT_THAT(my_list.Get(),
ElementsAre(Garment{1, TimeDelta::seconds(100)},
Garment{2, TimeDelta::PlusInfinity()}));
ElementsAre(Garment{1, "blue"}, Garment{2, "red"}));
}
// Some lists are provided and all are empty, so we return a empty list.
TEST(FieldTrialListTest, StructListHandlesEmptyLists) {
FieldTrialStructList<Garment> my_list(
{FieldTrialStructMember("age", [](Garment* g) { return &g->age; }),
{FieldTrialStructMember("color", [](Garment* g) { return &g->color; }),
FieldTrialStructMember("price", [](Garment* g) { return &g->price; })},
{{1, TimeDelta::seconds(100)}, {2, TimeDelta::PlusInfinity()}});
{{1, "blue"}, {2, "red"}});
ParseFieldTrial({&my_list}, "age,price");
ParseFieldTrial({&my_list}, "color,price");
ASSERT_EQ(my_list.Get().size(), 0u);
}

View File

@ -139,6 +139,11 @@ absl::optional<unsigned> ParseTypedParameter<unsigned>(std::string str) {
return absl::nullopt;
}
template <>
absl::optional<std::string> ParseTypedParameter<std::string>(std::string str) {
return std::move(str);
}
template <>
absl::optional<absl::optional<bool>> ParseTypedParameter<absl::optional<bool>>(
std::string str) {
@ -221,6 +226,7 @@ template class FieldTrialParameter<bool>;
template class FieldTrialParameter<double>;
template class FieldTrialParameter<int>;
template class FieldTrialParameter<unsigned>;
template class FieldTrialParameter<std::string>;
template class FieldTrialConstrained<double>;
template class FieldTrialConstrained<int>;
@ -230,5 +236,6 @@ template class FieldTrialOptional<double>;
template class FieldTrialOptional<int>;
template class FieldTrialOptional<unsigned>;
template class FieldTrialOptional<bool>;
template class FieldTrialOptional<std::string>;
} // namespace webrtc

View File

@ -244,9 +244,11 @@ template <>
absl::optional<double> ParseTypedParameter<double>(std::string str);
template <>
absl::optional<int> ParseTypedParameter<int>(std::string str);
template <>
absl::optional<unsigned> ParseTypedParameter<unsigned>(std::string str);
template <>
absl::optional<std::string> ParseTypedParameter<std::string>(std::string str);
template <>
absl::optional<absl::optional<bool>> ParseTypedParameter<absl::optional<bool>>(
std::string str);
@ -268,6 +270,8 @@ extern template class FieldTrialParameter<double>;
extern template class FieldTrialParameter<int>;
// Interpreted using sscanf %u.
extern template class FieldTrialParameter<unsigned>;
// Using the given value as is.
extern template class FieldTrialParameter<std::string>;
extern template class FieldTrialConstrained<double>;
extern template class FieldTrialConstrained<int>;
@ -277,6 +281,7 @@ extern template class FieldTrialOptional<double>;
extern template class FieldTrialOptional<int>;
extern template class FieldTrialOptional<unsigned>;
extern template class FieldTrialOptional<bool>;
extern template class FieldTrialOptional<std::string>;
} // namespace webrtc

View File

@ -25,13 +25,17 @@ struct DummyExperiment {
FieldTrialParameter<int> retries = FieldTrialParameter<int>("r", 5);
FieldTrialParameter<unsigned> size = FieldTrialParameter<unsigned>("s", 3);
FieldTrialParameter<bool> ping = FieldTrialParameter<bool>("p", 0);
FieldTrialParameter<std::string> hash =
FieldTrialParameter<std::string>("h", "a80");
explicit DummyExperiment(std::string field_trial) {
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping}, field_trial);
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash},
field_trial);
}
DummyExperiment() {
std::string trial_string = field_trial::FindFullName(kDummyExperiment);
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping}, trial_string);
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash},
trial_string);
}
};
@ -44,17 +48,18 @@ enum class CustomEnum {
} // namespace
TEST(FieldTrialParserTest, ParsesValidParameters) {
DummyExperiment exp("Enabled,f:-1.7,r:2,s:10,p:1");
DummyExperiment exp("Enabled,f:-1.7,r:2,s:10,p:1,h:x7c");
EXPECT_TRUE(exp.enabled.Get());
EXPECT_EQ(exp.factor.Get(), -1.7);
EXPECT_EQ(exp.retries.Get(), 2);
EXPECT_EQ(exp.size.Get(), 10u);
EXPECT_EQ(exp.ping.Get(), true);
EXPECT_EQ(exp.hash.Get(), "x7c");
}
TEST(FieldTrialParserTest, InitializesFromFieldTrial) {
test::ScopedFieldTrials field_trials(
"WebRTC-OtherExperiment/Disabled/"
"WebRTC-DummyExperiment/Enabled,f:-1.7,r:2,s:10,p:1/"
"WebRTC-DummyExperiment/Enabled,f:-1.7,r:2,s:10,p:1,h:x7c/"
"WebRTC-AnotherExperiment/Enabled,f:-3.1,otherstuff:beef/");
DummyExperiment exp;
EXPECT_TRUE(exp.enabled.Get());
@ -62,6 +67,7 @@ TEST(FieldTrialParserTest, InitializesFromFieldTrial) {
EXPECT_EQ(exp.retries.Get(), 2);
EXPECT_EQ(exp.size.Get(), 10u);
EXPECT_EQ(exp.ping.Get(), true);
EXPECT_EQ(exp.hash.Get(), "x7c");
}
TEST(FieldTrialParserTest, UsesDefaults) {
DummyExperiment exp("");
@ -70,6 +76,7 @@ TEST(FieldTrialParserTest, UsesDefaults) {
EXPECT_EQ(exp.retries.Get(), 5);
EXPECT_EQ(exp.size.Get(), 3u);
EXPECT_EQ(exp.ping.Get(), false);
EXPECT_EQ(exp.hash.Get(), "a80");
}
TEST(FieldTrialParserTest, CanHandleMixedInput) {
DummyExperiment exp("p:true,h:,Enabled");
@ -78,6 +85,7 @@ TEST(FieldTrialParserTest, CanHandleMixedInput) {
EXPECT_EQ(exp.retries.Get(), 5);
EXPECT_EQ(exp.size.Get(), 3u);
EXPECT_EQ(exp.ping.Get(), true);
EXPECT_EQ(exp.hash.Get(), "");
}
TEST(FieldTrialParserTest, ParsesDoubleParameter) {
FieldTrialParameter<double> double_param("f", 0.0);
@ -101,6 +109,7 @@ TEST(FieldTrialParserTest, IgnoresInvalid) {
EXPECT_EQ(exp.retries.Get(), 5);
EXPECT_EQ(exp.size.Get(), 3u);
EXPECT_EQ(exp.ping.Get(), false);
EXPECT_EQ(exp.hash.Get(), "a80");
}
TEST(FieldTrialParserTest, IgnoresOutOfRange) {
FieldTrialConstrained<double> low("low", 10, absl::nullopt, 100);
@ -150,6 +159,11 @@ TEST(FieldTrialParserTest, ParsesOptionalParameters) {
ParseFieldTrial({&max_size}, "c:20");
EXPECT_EQ(max_size.GetOptional().value(), 20u);
FieldTrialOptional<std::string> optional_string("s", std::string("ab"));
ParseFieldTrial({&optional_string}, "s:");
EXPECT_EQ(optional_string.GetOptional().value(), "");
ParseFieldTrial({&optional_string}, "s");
EXPECT_FALSE(optional_string.GetOptional().has_value());
}
TEST(FieldTrialParserTest, ParsesCustomEnumParameter) {
FieldTrialEnum<CustomEnum> my_enum("e", CustomEnum::kDefault,