From d6352f9f66f655f11332fe6c52ed71ebb2e55bf4 Mon Sep 17 00:00:00 2001
From: guillermo-bondonno <guillermo.bondonno@zyte.com>
Date: Wed, 10 Jul 2024 11:03:01 -0300
Subject: [PATCH] refactor changes on closespider.py and improve test

---
 scrapy/extensions/closespider.py | 18 +++---------------
 tests/spiders.py                 | 17 +++++++++++++++++
 tests/test_closespider.py        | 24 +++++++++++++++++++-----
 3 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py
index 6ebf98e65..cef5527b7 100644
--- a/scrapy/extensions/closespider.py
+++ b/scrapy/extensions/closespider.py
@@ -12,7 +12,6 @@ from typing import TYPE_CHECKING, Any, DefaultDict, Dict
 
 from scrapy import Request, Spider, signals
 from scrapy.exceptions import NotConfigured
-from scrapy.signalmanager import dispatcher
 
 if TYPE_CHECKING:
     from twisted.python.failure import Failure
@@ -49,11 +48,11 @@ class CloseSpider:
 
         if self.close_on.get("errorcount"):
             crawler.signals.connect(self.error_count, signal=signals.spider_error)
-        if self.close_on.get("pagecount"):
+        if self.close_on.get("pagecount") or self.close_on.get("pagecount_no_item"):
             crawler.signals.connect(self.page_count, signal=signals.response_received)
         if self.close_on.get("timeout"):
             crawler.signals.connect(self.spider_opened, signal=signals.spider_opened)
-        if self.close_on.get("itemcount"):
+        if self.close_on.get("itemcount") or self.close_on.get("pagecount_no_item"):
             crawler.signals.connect(self.item_scraped, signal=signals.item_scraped)
         if self.close_on.get("timeout_no_item"):
             self.timeout_no_item: int = self.close_on["timeout_no_item"]
@@ -64,18 +63,6 @@ class CloseSpider:
             crawler.signals.connect(
                 self.item_scraped_no_item, signal=signals.item_scraped
             )
-        if self.close_on.get("pagecount_no_item"):
-            if self.page_count not in dispatcher.getReceivers(
-                signal=signals.response_received
-            ):
-                crawler.signals.connect(
-                    self.page_count, signal=signals.response_received
-                )
-
-            if self.item_scraped not in dispatcher.getReceivers(
-                signal=signals.item_scraped
-            ):
-                crawler.signals.connect(self.item_scraped, signal=signals.item_scraped)
 
         crawler.signals.connect(self.spider_closed, signal=signals.spider_closed)
 
@@ -95,6 +82,7 @@ class CloseSpider:
         if self.counter["pagecount"] == self.close_on["pagecount"]:
             assert self.crawler.engine
             self.crawler.engine.close_spider(spider, "closespider_pagecount")
+            return
         if self.close_on["pagecount_no_item"] and (
             self.counter["pagecount_since_last_item"]
             >= self.close_on["pagecount_no_item"]
diff --git a/tests/spiders.py b/tests/spiders.py
index ea419afbd..2bcec5624 100644
--- a/tests/spiders.py
+++ b/tests/spiders.py
@@ -283,6 +283,23 @@ class ItemSpider(FollowAllSpider):
             yield {}
 
 
+class MaxItemsSpider(ItemSpider):
+    def __init__(self, max_items=10, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.max_items = max_items
+        self.items_scraped = 0
+
+    def parse(self, response):
+        for item_or_req in super().parse(response):
+            if isinstance(item_or_req, Request):
+                yield item_or_req
+            else:
+                if self.items_scraped >= self.max_items:
+                    continue
+                self.items_scraped += 1
+                yield item_or_req
+
+
 class DefaultError(Exception):
     pass
 
diff --git a/tests/test_closespider.py b/tests/test_closespider.py
index caaa9f183..0046b4e29 100644
--- a/tests/test_closespider.py
+++ b/tests/test_closespider.py
@@ -3,7 +3,13 @@ from twisted.trial.unittest import TestCase
 
 from scrapy.utils.test import get_crawler
 from tests.mockserver import MockServer
-from tests.spiders import ErrorSpider, FollowAllSpider, ItemSpider, SlowSpider
+from tests.spiders import (
+    ErrorSpider,
+    FollowAllSpider,
+    ItemSpider,
+    MaxItemsSpider,
+    SlowSpider,
+)
 
 
 class TestCloseSpider(TestCase):
@@ -37,15 +43,23 @@ class TestCloseSpider(TestCase):
     @defer.inlineCallbacks
     def test_closespider_pagecount_no_item(self):
         close_on = 5
+        close_on_pagecount = 20
+        max_items = 5
         crawler = get_crawler(
-            FollowAllSpider,
-            {"CLOSESPIDER_PAGECOUNT_NO_ITEM": close_on},
+            MaxItemsSpider,
+            {
+                "CLOSESPIDER_PAGECOUNT_NO_ITEM": close_on,
+                "CLOSESPIDER_PAGECOUNT": close_on_pagecount,
+            },
         )
-        yield crawler.crawl(mockserver=self.mockserver)
+        yield crawler.crawl(max_items=max_items, mockserver=self.mockserver)
         reason = crawler.spider.meta["close_reason"]
         self.assertEqual(reason, "closespider_pagecount_no_item")
         pagecount = crawler.stats.get_value("response_received_count")
-        self.assertTrue(pagecount >= close_on)
+        itemcount = crawler.stats.get_value("item_scraped_count")
+        self.assertEqual(itemcount, max_items)
+        self.assertLess(pagecount, close_on_pagecount)
+        self.assertTrue((pagecount - itemcount) >= close_on)
 
     @defer.inlineCallbacks
     def test_closespider_errorcount(self):