From c4f0aa4fdfff802c153b1eeda711957d063aed60 Mon Sep 17 00:00:00 2001 From: freddiewanah Date: Wed, 26 Jul 2023 21:09:03 +1000 Subject: [PATCH] Refactor test cases to improve unit test quality (#5986) --- tests/test_crawler.py | 8 ++++---- tests/test_downloader_handlers.py | 2 +- tests/test_downloadermiddleware_httpcompression.py | 6 +++--- tests/test_pipeline_files.py | 9 +++++++-- tests/test_pipeline_images.py | 10 ++++++++-- 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 68e58144b..f99606ccf 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -228,14 +228,14 @@ class CrawlerRunnerHasSpider(unittest.TestCase): def test_crawler_runner_bootstrap_successful(self): runner = self._runner() yield runner.crawl(NoRequestsSpider) - self.assertEqual(runner.bootstrap_failed, False) + self.assertFalse(runner.bootstrap_failed) @defer.inlineCallbacks def test_crawler_runner_bootstrap_successful_for_several(self): runner = self._runner() yield runner.crawl(NoRequestsSpider) yield runner.crawl(NoRequestsSpider) - self.assertEqual(runner.bootstrap_failed, False) + self.assertFalse(runner.bootstrap_failed) @defer.inlineCallbacks def test_crawler_runner_bootstrap_failed(self): @@ -248,7 +248,7 @@ class CrawlerRunnerHasSpider(unittest.TestCase): else: self.fail("Exception should be raised from spider") - self.assertEqual(runner.bootstrap_failed, True) + self.assertTrue(runner.bootstrap_failed) @defer.inlineCallbacks def test_crawler_runner_bootstrap_failed_for_several(self): @@ -263,7 +263,7 @@ class CrawlerRunnerHasSpider(unittest.TestCase): yield runner.crawl(NoRequestsSpider) - self.assertEqual(runner.bootstrap_failed, True) + self.assertTrue(runner.bootstrap_failed) @defer.inlineCallbacks def test_crawler_runner_asyncio_enabled_true(self): diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 9731b62c4..8459408ff 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -748,7 +748,7 @@ class Http11MockServerTestCase(unittest.TestCase): yield crawler.crawl(seed=request) # download_maxsize = 50 is enough for the gzipped response failure = crawler.spider.meta.get("failure") - self.assertTrue(failure is None) + self.assertIsNone(failure) reason = crawler.spider.meta["close_reason"] self.assertTrue(reason, "finished") diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index fac5588ff..9dad056de 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -229,7 +229,7 @@ class HttpCompressionTest(TestCase): self.assertEqual(newresponse.body, plainbody) self.assertEqual(newresponse.encoding, resolve_encoding("gb2312")) self.assertStatsEqual("httpcompression/response_count", 1) - self.assertStatsEqual("httpcompression/response_bytes", 104) + self.assertStatsEqual("httpcompression/response_bytes", len(plainbody)) def test_process_response_force_recalculate_encoding(self): headers = { @@ -254,7 +254,7 @@ class HttpCompressionTest(TestCase): self.assertEqual(newresponse.body, plainbody) self.assertEqual(newresponse.encoding, resolve_encoding("gb2312")) self.assertStatsEqual("httpcompression/response_count", 1) - self.assertStatsEqual("httpcompression/response_bytes", 104) + self.assertStatsEqual("httpcompression/response_bytes", len(plainbody)) def test_process_response_no_content_type_header(self): headers = { @@ -277,7 +277,7 @@ class HttpCompressionTest(TestCase): self.assertEqual(newresponse.body, plainbody) self.assertEqual(newresponse.encoding, resolve_encoding("gb2312")) self.assertStatsEqual("httpcompression/response_count", 1) - self.assertStatsEqual("httpcompression/response_bytes", 104) + self.assertStatsEqual("httpcompression/response_bytes", len(plainbody)) def test_process_response_gzipped_contenttype(self): response = self._getresponse("gzip") diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index fe7b26740..bf96f17b6 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -468,8 +468,13 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): pipeline = UserDefinedFilesPipeline.from_settings( Settings({"FILES_STORE": self.tempdir}) ) - self.assertEqual(pipeline.files_result_field, "this") - self.assertEqual(pipeline.files_urls_field, "that") + self.assertEqual( + pipeline.files_result_field, + UserDefinedFilesPipeline.DEFAULT_FILES_RESULT_FIELD, + ) + self.assertEqual( + pipeline.files_urls_field, UserDefinedFilesPipeline.DEFAULT_FILES_URLS_FIELD + ) def test_user_defined_subclass_default_key_names(self): """Test situation when user defines subclass of FilesPipeline, diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 6fd8e6308..8924875d1 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -598,8 +598,14 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): pipeline = UserDefinedImagePipeline.from_settings( Settings({"IMAGES_STORE": self.tempdir}) ) - self.assertEqual(pipeline.images_result_field, "something_else") - self.assertEqual(pipeline.images_urls_field, "something") + self.assertEqual( + pipeline.images_result_field, + UserDefinedImagePipeline.DEFAULT_IMAGES_RESULT_FIELD, + ) + self.assertEqual( + pipeline.images_urls_field, + UserDefinedImagePipeline.DEFAULT_IMAGES_URLS_FIELD, + ) def test_user_defined_subclass_default_key_names(self): """Test situation when user defines subclass of ImagePipeline,