Skip to content

Commit

Permalink
Fix BatAdsAdNotificationServingTest.ServeAdWithAdServingVersion2 setup
Browse files Browse the repository at this point in the history
This test was not being properly setup since a string was expected to
be set as the value for ad_serving_parameters["ad_serving_version"],
but instead an integer (2, instead of "2") was being passed.

And we didn't noticed until now because the parsing problem would go
unnoticed as a DLOG(WARNING), resulting in the default value being
used, but now that the DLOG(WARNING) became a NOTREACHED() the test
is crashing instead.

To fix this, we just make sure we actually set a string as the value
for ad_serving_parameters["ad_serving_version"], and also add a couple
of checks more in the test suite to make sure that AdServing is enabled
and that the version being used is, indeed, version 2.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/bf076ce0d82267340f9ba6aa4849d8a37aa8a0db

commit bf076ce0d82267340f9ba6aa4849d8a37aa8a0db
Author: Christian Dullweber <[email protected]>
Date:   Wed Oct 20 10:35:26 2021 +0000

    AccuracyTips: Fix field_trial config and add DCHECKs to config parser

    FeatureParam<TimeDelta> does not support days as parameter value, so our
    "1d" config is not parsed correctly and instead 7 days are used.
    Fix the config by using hours and throw errors when an incorrect config
    is parsed instead of just logging a warning to avoid future issues.

    Bug: 1261162
  • Loading branch information
mariospr committed Oct 27, 2021
1 parent c34b479 commit 57d38f9
Showing 1 changed file with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,15 @@ TEST_F(BatAdsAdNotificationServingTest, ServeAdWithAdServingVersion2) {
&anti_targeting_resource);

std::map<std::string, std::string> ad_serving_parameters;
ad_serving_parameters["ad_serving_version"] = 2;
ad_serving_parameters["ad_serving_version"] = "2";

base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeaturesAndParameters(
{{features::kAdServing, ad_serving_parameters}}, {});

ASSERT_TRUE(features::IsAdServingEnabled());
ASSERT_EQ(2, features::GetAdServingVersion());

// Act
EXPECT_CALL(*ads_client_mock_, ShowNotification(DoesMatchCreativeInstanceId(
creative_ad.creative_instance_id)))
Expand Down

0 comments on commit 57d38f9

Please sign in to comment.