From 10850e7d2933506f4715e86f8cd153534d601498 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 6 Oct 2025 14:19:34 +0500 Subject: [PATCH] Fix extra stdout output in tests (#7070) --- scrapy/utils/log.py | 13 +++++++--- tests/test_command_startproject.py | 16 ++++++++----- tests/test_crawler.py | 38 +++++++++++++++++++----------- tests/test_engine.py | 1 + tests/test_feedexport.py | 2 -- tests/test_proxy_connect.py | 2 -- tests/test_zz_resources.py | 15 ++++++++++-- 7 files changed, 58 insertions(+), 29 deletions(-) diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 533906003..e45d34ac5 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -134,14 +134,21 @@ _scrapy_root_handler: logging.Handler | None = None def install_scrapy_root_handler(settings: Settings) -> None: global _scrapy_root_handler # noqa: PLW0603 # pylint: disable=global-statement + _uninstall_scrapy_root_handler() + logging.root.setLevel(logging.NOTSET) + _scrapy_root_handler = _get_handler(settings) + logging.root.addHandler(_scrapy_root_handler) + + +def _uninstall_scrapy_root_handler() -> None: + global _scrapy_root_handler # noqa: PLW0603 # pylint: disable=global-statement + if ( _scrapy_root_handler is not None and _scrapy_root_handler in logging.root.handlers ): logging.root.removeHandler(_scrapy_root_handler) - logging.root.setLevel(logging.NOTSET) - _scrapy_root_handler = _get_handler(settings) - logging.root.addHandler(_scrapy_root_handler) + _scrapy_root_handler = None def get_scrapy_root_handler() -> logging.Handler | None: diff --git a/tests/test_command_startproject.py b/tests/test_command_startproject.py index 1edef0b4a..246066485 100644 --- a/tests/test_command_startproject.py +++ b/tests/test_command_startproject.py @@ -17,9 +17,7 @@ from tests.test_commands import TestProjectBase class TestStartprojectCommand(TestProjectBase): def test_startproject(self): - p, out, err = self.proc("startproject", self.project_name) - print(out) - print(err, file=sys.stderr) + p, _, _ = self.proc("startproject", self.project_name) assert p.returncode == 0 assert Path(self.proj_path, "scrapy.cfg").exists() @@ -64,9 +62,7 @@ class TestStartprojectCommand(TestProjectBase): project_path = Path(project_dir, project_name) project_path.mkdir() - p, out, err = self.proc("startproject", project_name, cwd=project_dir) - print(out) - print(err, file=sys.stderr) + p, _, _ = self.proc("startproject", project_name, cwd=project_dir) assert p.returncode == 0 assert Path(project_path, "scrapy.cfg").exists() @@ -151,6 +147,8 @@ class TestStartprojectTemplates(TestProjectBase): project_name, ), cwd=destination, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, env=self.env, ) process.wait() @@ -204,6 +202,8 @@ class TestStartprojectTemplates(TestProjectBase): f"TEMPLATES_DIR={read_only_templates_dir}", ), cwd=destination, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, env=self.env, ) process.wait() @@ -263,6 +263,8 @@ class TestStartprojectTemplates(TestProjectBase): ".", ), cwd=project_dir, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, env=self.env, ) process.wait() @@ -306,6 +308,8 @@ class TestStartprojectTemplates(TestProjectBase): project_name, ), cwd=destination, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, env=self.env, ) process.wait() diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 66383b92b..2be3b49b3 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -36,7 +36,11 @@ from scrapy.utils.defer import ( deferred_from_coro, maybe_deferred_to_future, ) -from scrapy.utils.log import configure_logging, get_scrapy_root_handler +from scrapy.utils.log import ( + _uninstall_scrapy_root_handler, + configure_logging, + get_scrapy_root_handler, +) from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler, get_reactor_settings from tests.mockserver.http import MockServer @@ -543,11 +547,14 @@ class TestCrawlerLogging: return yield - configure_logging() - assert get_scrapy_root_handler().level == logging.DEBUG - crawler = get_crawler(MySpider) - assert get_scrapy_root_handler().level == logging.INFO - await maybe_deferred_to_future(crawler.crawl()) + try: + configure_logging() + assert get_scrapy_root_handler().level == logging.DEBUG + crawler = get_crawler(MySpider) + assert get_scrapy_root_handler().level == logging.INFO + await maybe_deferred_to_future(crawler.crawl()) + finally: + _uninstall_scrapy_root_handler() logged = log_file.read_text(encoding="utf-8") @@ -572,9 +579,12 @@ class TestCrawlerLogging: "LOG_FILE_APPEND": False, } - configure_logging() - get_crawler(MySpider) - logging.debug("debug message") # noqa: LOG015 + try: + configure_logging() + get_crawler(MySpider) + logging.debug("debug message") # noqa: LOG015 + finally: + _uninstall_scrapy_root_handler() logged = log_file.read_text(encoding="utf-8") @@ -629,24 +639,24 @@ class TestAsyncCrawlerRunner(TestBaseCrawler): class TestCrawlerProcess(TestBaseCrawler): def test_crawler_process_accepts_dict(self): - runner = CrawlerProcess({"foo": "bar"}) + runner = CrawlerProcess({"foo": "bar"}, install_root_handler=False) assert runner.settings["foo"] == "bar" self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED") def test_crawler_process_accepts_None(self): - runner = CrawlerProcess() + runner = CrawlerProcess(install_root_handler=False) self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED") @pytest.mark.only_asyncio class TestAsyncCrawlerProcess(TestBaseCrawler): def test_crawler_process_accepts_dict(self): - runner = AsyncCrawlerProcess({"foo": "bar"}) + runner = AsyncCrawlerProcess({"foo": "bar"}, install_root_handler=False) assert runner.settings["foo"] == "bar" self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED") def test_crawler_process_accepts_None(self): - runner = AsyncCrawlerProcess() + runner = AsyncCrawlerProcess(install_root_handler=False) self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED") @@ -1177,7 +1187,7 @@ class TestAsyncCrawlerRunnerSubprocess(TestCrawlerRunnerSubprocessBase): ) def test_log_scrapy_info(settings, items, caplog): with caplog.at_level("INFO"): - CrawlerProcess(settings) + CrawlerProcess(settings, install_root_handler=False) assert ( caplog.records[0].getMessage() == f"Scrapy {scrapy.__version__} started (bot: scrapybot)" diff --git a/tests/test_engine.py b/tests/test_engine.py index 4bfccdb18..430d599c8 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -489,6 +489,7 @@ class TestEngine(TestEngineBase): ) p = subprocess.Popen( args, + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, ) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index c5bd1b172..aa4477210 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1717,7 +1717,6 @@ class TestFeedExport(TestFeedExportBase): with LogCapture() as log: await self.exported_data(items, settings) - print(log) for fmt in ["json", "xml", "csv"]: assert f"Stored {fmt} feed (2 items)" in str(log) @@ -1738,7 +1737,6 @@ class TestFeedExport(TestFeedExportBase): with LogCapture() as log: await self.exported_data(items, settings) - print(log) for fmt in ["json", "xml", "csv"]: assert f"Error storing {fmt} feed (2 items)" in str(log) diff --git a/tests/test_proxy_connect.py b/tests/test_proxy_connect.py index f9874f821..61d79743d 100644 --- a/tests/test_proxy_connect.py +++ b/tests/test_proxy_connect.py @@ -116,9 +116,7 @@ class TestProxyConnect: assert "Proxy-Authorization" not in echo["headers"] def _assert_got_response_code(self, code, log): - print(log) assert str(log).count(f"Crawled ({code})") == 1 def _assert_got_tunnel_error(self, log): - print(log) assert "TunnelError" in str(log) diff --git a/tests/test_zz_resources.py b/tests/test_zz_resources.py index 84b2910db..2560f8d7f 100644 --- a/tests/test_zz_resources.py +++ b/tests/test_zz_resources.py @@ -8,9 +8,20 @@ from scrapy.utils.log import LogCounterHandler def test_counter_handler() -> None: - """Test that LogCounterHandler is always properly removed. + """Test that ``LogCounterHandler`` is always properly removed. - It's added in Crawler.crawl{,_async}() and removed on engine_stopped. + It's added in ``Crawler.crawl{,_async}()`` and removed on engine_stopped. """ c = sum(1 for h in logging.root.handlers if isinstance(h, LogCounterHandler)) assert c == 0 + + +def test_stderr_log_handler() -> None: + """Test that the Scrapy root handler is always properly removed. + + It's added in ``configure_logging()``, called by ``{Async,}CrawlerProcess`` + (without ``install_root_handler=False``). It can be removed with + ``_uninstall_scrapy_root_handler()`` if installing it was really neeeded. + """ + c = sum(1 for h in logging.root.handlers if type(h) is logging.StreamHandler) # pylint: disable=unidiomatic-typecheck + assert c == 0