[ads] Unit test code health

brave · Nov 7, 2024 · 9e0e608 · 9e0e608
1 parent 6982b6b
commit 9e0e608
Show file tree

Hide file tree

Showing 9 changed files with 141 additions and 75 deletions.
diff --git a/components/brave_ads/core/internal/ad_units/creative_ad_cache_unittest.cc b/components/brave_ads/core/internal/ad_units/creative_ad_cache_unittest.cc
@@ -10,6 +10,7 @@
 #include "brave/components/brave_ads/core/internal/ad_units/ad_test_constants.h"
 #include "brave/components/brave_ads/core/internal/common/test/test_base.h"
 #include "brave/components/brave_ads/core/internal/creatives/search_result_ads/creative_search_result_ad_test_util.h"
+#include "net/http/http_status_code.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 // npm run test -- brave_unit_tests --filter=BraveAds*
@@ -34,7 +35,8 @@ TEST_F(BraveAdsCreativeAdCacheTest, AddCreativeAd) {
           /*should_generate_random_uuids=*/true);
 
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   // Act
   creative_ad_cache_->MaybeAdd(test::kPlacementId, mojom_creative_ad->Clone());
@@ -48,7 +50,8 @@ TEST_F(BraveAdsCreativeAdCacheTest, AddCreativeAd) {
 TEST_F(BraveAdsCreativeAdCacheTest, DoNotAddCreativeAd) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   // Act
   creative_ad_cache_->MaybeAdd(test::kPlacementId,
@@ -63,7 +66,8 @@ TEST_F(BraveAdsCreativeAdCacheTest, DoNotAddCreativeAd) {
 TEST_F(BraveAdsCreativeAdCacheTest, DoNotAddInvalidCreativeAd) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   CreativeAdVariant creative_ad_variant;
 
@@ -117,11 +121,13 @@ TEST_F(BraveAdsCreativeAdCacheTest, GetCreativeAdsAcrossMultipleTabs) {
 
   // Act
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
   creative_ad_cache_->MaybeAdd(test::kPlacementId, mojom_creative_ad->Clone());
 
   SimulateOpeningNewTab(/*tab_id=*/2,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
   creative_ad_cache_->MaybeAdd(test::kAnotherPlacementId,
                                mojom_creative_ad->Clone());
 
@@ -141,11 +147,13 @@ TEST_F(BraveAdsCreativeAdCacheTest, PurgePlacementsOnTabDidClose) {
           /*should_generate_random_uuids=*/true);
 
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
   creative_ad_cache_->MaybeAdd(test::kPlacementId, mojom_creative_ad->Clone());
 
   SimulateOpeningNewTab(/*tab_id=*/2,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
   creative_ad_cache_->MaybeAdd(test::kAnotherPlacementId,
                                mojom_creative_ad->Clone());
 

diff --git a/components/brave_ads/core/internal/ad_units/inline_content_ad/inline_content_ad_test.cc b/components/brave_ads/core/internal/ad_units/inline_content_ad/inline_content_ad_test.cc
@@ -33,7 +33,8 @@ class BraveAdsInlineContentAdIntegrationTest : public test::TestBase {
     test::TestBase::SetUp(/*is_integration_test=*/true);
 
     SimulateOpeningNewTab(/*tab_id=*/1,
-                          /*redirect_chain=*/{GURL("brave://newtab")});
+                          /*redirect_chain=*/{GURL("brave://newtab")},
+                          net::HTTP_OK);
   }
 
   void SetUpMocks() override {

diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.cc b/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.cc
@@ -7,7 +7,6 @@
 
 #include "base/check_op.h"
 #include "base/containers/contains.h"
-#include "net/http/http_status_code.h"
 #include "url/gurl.h"
 
 namespace brave_ads {
@@ -177,19 +176,21 @@ void AdsClientNotifierForTesting::NotifyDidSolveAdaptiveCaptcha() {
 
 void AdsClientNotifierForTesting::SimulateOpeningNewTab(
     const int32_t tab_id,
-    const std::vector<GURL>& redirect_chain) {
+    const std::vector<GURL>& redirect_chain,
+    const int http_status_code) {
   CHECK(!base::Contains(redirect_chains_, tab_id)) << "Tab already open";
 
   redirect_chains_[tab_id] = redirect_chain;
 
   SimulateSelectTab(tab_id);
 
-  SimulateNavigateToURL(tab_id, redirect_chain);
+  SimulateNavigateToURL(tab_id, redirect_chain, http_status_code);
 }
 
 void AdsClientNotifierForTesting::SimulateNavigateToURL(
     const int32_t tab_id,
-    const std::vector<GURL>& redirect_chain) {
+    const std::vector<GURL>& redirect_chain,
+    const int http_status_code) {
   CHECK(base::Contains(redirect_chains_, tab_id)) << "Tab does not exist";
 
   redirect_chains_[tab_id] = redirect_chain;
@@ -198,7 +199,7 @@ void AdsClientNotifierForTesting::SimulateNavigateToURL(
 
   NotifyTabDidChange(tab_id, redirect_chain, /*is_new_navigation=*/true,
                      /*is_restoring=*/false, is_visible);
-  NotifyTabDidLoad(tab_id, net::HTTP_OK);
+  NotifyTabDidLoad(tab_id, http_status_code);
 }
 
 void AdsClientNotifierForTesting::SimulateSelectTab(const int32_t tab_id) {

diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.h b/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.h
@@ -94,9 +94,11 @@ class AdsClientNotifierForTesting : public AdsClientNotifier {
 
   // Simulate helper functions.
   void SimulateOpeningNewTab(int32_t tab_id,
-                             const std::vector<GURL>& redirect_chain);
+                             const std::vector<GURL>& redirect_chain,
+                             int http_status_code);
   void SimulateNavigateToURL(int32_t tab_id,
-                             const std::vector<GURL>& redirect_chain);
+                             const std::vector<GURL>& redirect_chain,
+                             int http_status_code);
   void SimulateSelectTab(int32_t tab_id);
   void SimulateClosingTab(int32_t tab_id);
 

diff --git a/components/brave_ads/core/internal/common/net/http/http_status_code_util_unittest.cc b/components/brave_ads/core/internal/common/net/http/http_status_code_util_unittest.cc
@@ -5,37 +5,43 @@
 
 #include "brave/components/brave_ads/core/internal/common/net/http/http_status_code_util.h"
 
+#include "base/containers/fixed_flat_set.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "net/http/http_status_code.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 // npm run test -- brave_unit_tests --filter=BraveAds*
 
 namespace brave_ads {
 
 TEST(BraveAdsHttpStatusCodeUtilTest, HttpStatusCodeToString) {
+  // Arrange
+  static constexpr auto kAllowedHttpStatusCodes = base::MakeFixedFlatSet<int>({
+      400,  // Bad Request.
+      401,  // Unauthorized.
+      403,  // Forbidden.
+      404,  // Not Found.
+      407,  // Proxy Authentication Required.
+      408,  // Request Timeout.
+      429,  // Too Many Requests.
+      451,  // Unavailable For Legal Reasons.
+      500,  // Internal Server Error.
+      502,  // Bad Gateway.
+      503,  // Service Unavailable.
+      504   // Gateway Timeout.
+  });
+
   // Act & Assert
-  for (int i = 0; i <= 1024; ++i) {
+  for (int i = 0; i <= net::HTTP_STATUS_CODE_MAX; ++i) {
     const std::optional<std::string> http_status_code =
         HttpStatusCodeToString(i);
     if (!http_status_code) {
       // Nonsensical HTTP status code.
       continue;
     }
 
-    // Allowed HTTP status codes, other codes are mapped to their class.
-    if (i == 400 ||  // Bad Request.
-        i == 401 ||  // Unauthorized.
-        i == 403 ||  // Forbidden.
-        i == 404 ||  // Not Found.
-        i == 407 ||  // Proxy Authentication Required.
-        i == 408 ||  // Request Timeout.
-        i == 429 ||  // Too Many Requests.
-        i == 451 ||  // Unavailable For Legal Reasons.
-        i == 500 ||  // Internal Server Error.
-        i == 502 ||  // Bad Gateway.
-        i == 503 ||  // Service Unavailable.
-        i == 504) {  // Gateway Timeout.
+    if (kAllowedHttpStatusCodes.contains(i)) {
       EXPECT_EQ(base::NumberToString(i), http_status_code);
     } else {
       EXPECT_EQ(base::StringPrintf("%dxx", /*http_status_code_class*/ i / 100),

diff --git a/components/brave_ads/core/internal/serving/inline_content_ad_serving_unittest.cc b/components/brave_ads/core/internal/serving/inline_content_ad_serving_unittest.cc
@@ -20,6 +20,7 @@
 #include "brave/components/brave_ads/core/internal/targeting/behavioral/anti_targeting/resource/anti_targeting_resource.h"
 #include "brave/components/brave_ads/core/internal/targeting/geographical/subdivision/subdivision_targeting.h"
 #include "brave/components/brave_ads/core/public/ad_units/inline_content_ad/inline_content_ad_info.h"
+#include "net/http/http_status_code.h"
 
 // npm run test -- brave_unit_tests --filter=BraveAds*
 
@@ -30,7 +31,8 @@ class BraveAdsInlineContentAdServingTest : public test::TestBase {
   void MaybeServeAd(const std::string& dimensions,
                     MaybeServeInlineContentAdCallback callback) {
     SimulateOpeningNewTab(/*tab_id=*/1,
-                          /*redirect_chain=*/{GURL("brave://newtab")});
+                          /*redirect_chain=*/{GURL("brave://newtab")},
+                          net::HTTP_OK);
 
     SubdivisionTargeting subdivision_targeting;
     AntiTargetingResource anti_targeting_resource;

diff --git a/...onents/brave_ads/core/internal/serving/permission_rules/media_permission_rule_unittest.cc b/...onents/brave_ads/core/internal/serving/permission_rules/media_permission_rule_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/test/scoped_feature_list.h"
 #include "brave/components/brave_ads/core/internal/common/test/test_base.h"
 #include "brave/components/brave_ads/core/internal/serving/permission_rules/permission_rule_feature.h"
+#include "net/http/http_status_code.h"
 #include "url/gurl.h"
 
 // npm run test -- brave_unit_tests --filter=BraveAds*
@@ -25,7 +26,8 @@ TEST_F(BraveAdsMediaPermissionRuleTest,
        ShouldAllowIfMediaIsStoppedForSingleTab) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
 
@@ -39,7 +41,8 @@ TEST_F(BraveAdsMediaPermissionRuleTest,
        ShouldAllowIfMediaIsStoppedOnMultipleTabs) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
   NotifyTabDidStartPlayingMedia(/*tab_id=*/2);
@@ -55,7 +58,8 @@ TEST_F(BraveAdsMediaPermissionRuleTest,
        ShouldAllowIfMediaIsPlayingOnMultipleTabsButStoppedForVisibleTab) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
   NotifyTabDidStartPlayingMedia(/*tab_id=*/2);
@@ -70,7 +74,8 @@ TEST_F(BraveAdsMediaPermissionRuleTest,
        ShouldNotAllowIfMediaIsPlayingOnVisibleTab) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
 
@@ -88,7 +93,8 @@ TEST_F(
       {{"should_only_serve_ads_if_media_is_not_playing", "false"}});
 
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
 
@@ -100,7 +106,8 @@ TEST_F(BraveAdsMediaPermissionRuleTest,
        ShouldNotAllowIfMediaIsPlayingOnMultipleTabs) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
   NotifyTabDidStartPlayingMedia(/*tab_id=*/2);
@@ -113,7 +120,8 @@ TEST_F(BraveAdsMediaPermissionRuleTest,
        ShouldNotAllowIfMediaIsPlayingOnMultipleTabsButStoppedForOccludedTab) {
   // Arrange
   SimulateOpeningNewTab(/*tab_id=*/1,
-                        /*redirect_chain=*/{GURL("https://brave.com")});
+                        /*redirect_chain=*/{GURL("https://brave.com")},
+                        net::HTTP_OK);
 
   NotifyTabDidStartPlayingMedia(/*tab_id=*/1);
   NotifyTabDidStartPlayingMedia(/*tab_id=*/2);