Add tests for request backout

scrapy/core/downloader/__init__.py

@@ -175,7 +175,7 @@ class Downloader:
 
     def needs_backout(self) -> bool:
         if len(self.active) >= self.total_concurrency:
-            self._count_backout("concurrent_requests")
+            self._count_backout("concurrency")
             return True
         if (
             self._response_max_active_size

scrapy/extensions/corestats.py

@@ -49,11 +49,12 @@ class CoreStats:
         self.stats.set_value("finish_reason", reason, spider=spider)
         if elapsed_time_seconds > 0:
             request_backouts = self.stats.get_value("request_backouts/total", 0)
-            self.stats.set_value(
-                "request_backouts/total_per_second",
-                request_backouts / elapsed_time_seconds,
-                spider=spider,
-            )
+            if request_backouts:
+                self.stats.set_value(
+                    "request_backouts/total_per_second",
+                    request_backouts / elapsed_time_seconds,
+                    spider=spider,
+                )
 
     def item_scraped(self, item: Any, spider: Spider) -> None:
         self.stats.inc_value("item_scraped_count", spider=spider)

tests/test_downloader.py

@@ -27,6 +27,18 @@ class OfflineSpider(Spider):
         pass
 
 
+class gt:
+
+    def __init__(self, value):
+        self.value = value
+
+    def __eq__(self, other):
+        return other > self.value
+
+    def __repr__(self):
+        return f">{self.value}"
+
+
 class ResponseMaxActiveSizeTest(unittest.TestCase):
 
     @deferred_f_from_coro_f
@@ -114,3 +126,169 @@ class ResponseMaxActiveSizeTest(unittest.TestCase):
                 "RESPONSE_MAX_ACTIVE_SIZE instead."
             ),
         )
+
+    @deferred_f_from_coro_f
+    async def test_both_deprecated_priority(self):
+        """Setting RESPONSE_MAX_ACTIVE_SIZE and SCRAPER_SLOT_MAX_ACTIVE_SIZE to
+        different values and SCRAPER_SLOT_MAX_ACTIVE_SIZE with a higher
+        priority triggers a deprecation warning about
+        SCRAPER_SLOT_MAX_ACTIVE_SIZE but also makes the value of
+        SCRAPER_SLOT_MAX_ACTIVE_SIZE the effective response max active size."""
+
+        class TestSpider(Spider):
+            name = "test"
+            start_urls = ["data:,"]
+
+            @classmethod
+            def update_settings(cls, settings):
+                settings.set("RESPONSE_MAX_ACTIVE_SIZE", 1, priority=100)
+                settings.set("SCRAPER_SLOT_MAX_ACTIVE_SIZE", 2, priority=101)
+
+            def parse(self, response):
+                pass
+
+        crawler = get_crawler(TestSpider)
+        with pytest.warns(ScrapyDeprecationWarning) as warning_messages:
+            await crawler.crawl()
+        self.assertEqual(crawler.engine.downloader._response_max_active_size, 2)
+        self.assertEqual(len(warning_messages), 1)
+        self.assertEqual(
+            str(warning_messages[0].message),
+            (
+                "The SCRAPER_SLOT_MAX_ACTIVE_SIZE setting is deprecated, use "
+                "RESPONSE_MAX_ACTIVE_SIZE instead."
+            ),
+        )
+
+
+class RequestBackoutTest(unittest.TestCase):
+
+    @pytest.fixture(autouse=True)
+    def use_caplog(self, caplog):
+        self.caplog = caplog
+
+    @deferred_f_from_coro_f
+    async def test_none(self):
+
+        class TestSpider(Spider):
+            name = "test"
+            start_urls = ["data:,"]
+
+            def parse(self, response):
+                pass
+
+        crawler = get_crawler(TestSpider)
+        self.caplog.clear()
+        with self.caplog.at_level("INFO"):
+            await crawler.crawl()
+
+        matching_log_count = 0
+        for log_record in self.caplog.records:
+            if (
+                str(log_record.msg).startswith("The active response size")
+                and log_record.levelname == "INFO"
+            ):
+                matching_log_count += 1
+        self.assertEqual(matching_log_count, 0)
+
+        stats = {
+            k: v
+            for k, v in crawler.stats.get_stats().items()
+            if k.startswith("request_backouts/")
+        }
+        self.assertEqual(stats, {})
+
+    @deferred_f_from_coro_f
+    async def test_concurrency(self):
+
+        class SlowDown:
+            """Downloader middleware that returns a non-instant deferred from
+            process_request, to force need_backout calls to happen at that
+            point."""
+
+            def process_request(self, request, spider):
+                from twisted.internet import reactor
+                from twisted.internet.defer import Deferred
+
+                d = Deferred()
+                reactor.callLater(0, d.callback, None)
+                return d
+
+        class TestSpider(Spider):
+            name = "test"
+            start_urls = ["data:,"]
+            custom_settings = {
+                "CONCURRENT_REQUESTS": 1,
+                "DOWNLOADER_MIDDLEWARES": {SlowDown: 0},
+            }
+
+            def parse(self, response):
+                pass
+
+        crawler = get_crawler(TestSpider)
+        self.caplog.clear()
+        with self.caplog.at_level("INFO"):
+            await crawler.crawl()
+
+        matching_log_count = 0
+        for log_record in self.caplog.records:
+            if (
+                str(log_record.msg).startswith("The active response size")
+                and log_record.levelname == "INFO"
+            ):
+                matching_log_count += 1
+        self.assertEqual(matching_log_count, 0)
+
+        expected_stats = {
+            "request_backouts/concurrency": gt(0),
+            "request_backouts/total": gt(0),
+            "request_backouts/total_per_second": gt(0),
+        }
+        actual_stats = {
+            k: v
+            for k, v in crawler.stats.get_stats().items()
+            if k.startswith("request_backouts/")
+        }
+        self.assertEqual(expected_stats, actual_stats)
+
+    @deferred_f_from_coro_f
+    async def test_response_size(self):
+
+        class TestSpider(Spider):
+            name = "test"
+            start_urls = ["data:,a"]
+            custom_settings = {
+                "RESPONSE_MAX_ACTIVE_SIZE": 1,
+            }
+
+            def parse(self, response):
+                pass
+
+        crawler = get_crawler(TestSpider)
+        self.caplog.clear()
+        with self.caplog.at_level("INFO"):
+            await crawler.crawl()
+
+        matching_log_count = 0
+        for log_record in self.caplog.records:
+            if (
+                str(log_record.msg).startswith("The active response size")
+                and log_record.levelname == "INFO"
+            ):
+                matching_log_count += 1
+        self.assertEqual(matching_log_count, 1)
+
+        expected_stats = {
+            # Test > 1, if 1 then we are not really making sure that the INFO
+            # message above is logged only once in a scenario where active size
+            # is checked more than once.
+            "request_backouts/response_max_active_size": gt(1),
+            "request_backouts/total": gt(0),
+            "request_backouts/total_per_second": gt(0),
+        }
+        actual_stats = {
+            k: v
+            for k, v in crawler.stats.get_stats().items()
+            if k.startswith("request_backouts/")
+        }
+        self.assertEqual(expected_stats, actual_stats)