From 6f8758624c8d3df7472948ce9805601f6037548a Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 11 May 2020 13:50:34 -0300 Subject: [PATCH 1/5] Flake8: remove F841 --- pytest.ini | 22 ++++++++++------------ tests/pipelines.py | 2 +- tests/test_crawler.py | 6 +++--- tests/test_dependencies.py | 2 +- tests/test_extension_telnet.py | 2 -- tests/test_feedexport.py | 1 - tests/test_item.py | 8 ++++---- tests/test_pipeline_images.py | 2 +- tests/test_spidermiddleware_referer.py | 3 +-- tests/test_utils_defer.py | 4 ++-- tests/test_utils_deprecate.py | 4 ++-- tests/test_utils_signal.py | 2 +- 12 files changed, 26 insertions(+), 32 deletions(-) diff --git a/pytest.ini b/pytest.ini index 32fd76445..fee54dcbd 100644 --- a/pytest.ini +++ b/pytest.ini @@ -164,14 +164,13 @@ flake8-ignore = scrapy/robotstxt.py E501 scrapy/shell.py E501 scrapy/signalmanager.py E501 - scrapy/spiderloader.py F841 E501 + scrapy/spiderloader.py E501 scrapy/squeues.py E128 scrapy/squeues.py E501 scrapy/statscollectors.py E501 # tests tests/__init__.py E402 E501 tests/mockserver.py E501 - tests/pipelines.py F841 tests/spiders.py E501 tests/test_closespider.py E501 tests/test_command_fetch.py E501 @@ -180,8 +179,8 @@ flake8-ignore = tests/test_commands.py E128 E501 tests/test_contracts.py E501 E128 tests/test_crawl.py E501 E741 - tests/test_crawler.py F841 E501 - tests/test_dependencies.py F841 E501 + tests/test_crawler.py E501 + tests/test_dependencies.py E501 tests/test_downloader_handlers.py E128 E501 tests/test_downloadermiddleware.py E501 tests/test_downloadermiddleware_ajaxcrawlable.py E501 @@ -199,13 +198,12 @@ flake8-ignore = tests/test_dupefilters.py E501 E741 E128 tests/test_engine.py E501 E128 tests/test_exporters.py E501 E128 - tests/test_extension_telnet.py F841 - tests/test_feedexport.py E501 F841 + tests/test_feedexport.py E501 tests/test_http_cookies.py E501 tests/test_http_headers.py E501 tests/test_http_request.py E402 E501 E128 E128 tests/test_http_response.py E501 E128 - tests/test_item.py E128 F841 + tests/test_item.py E128 tests/test_link.py E501 tests/test_linkextractors.py E501 E128 tests/test_loader.py E501 E741 E128 @@ -214,7 +212,7 @@ flake8-ignore = tests/test_middleware.py E501 E128 tests/test_pipeline_crawl.py E501 E128 tests/test_pipeline_files.py E501 - tests/test_pipeline_images.py F841 E501 + tests/test_pipeline_images.py E501 tests/test_pipeline_media.py E501 E741 E128 tests/test_proxy_connect.py E501 E741 tests/test_request_cb_kwargs.py E501 @@ -227,14 +225,14 @@ flake8-ignore = tests/test_spidermiddleware_httperror.py E128 E501 tests/test_spidermiddleware_offsite.py E501 E128 tests/test_spidermiddleware_output_chain.py E501 - tests/test_spidermiddleware_referer.py E501 F841 E501 + tests/test_spidermiddleware_referer.py E501 E501 tests/test_squeues.py E501 E741 tests/test_utils_asyncio.py E501 tests/test_utils_conf.py E501 E128 tests/test_utils_curl.py E501 tests/test_utils_datatypes.py E402 E501 - tests/test_utils_defer.py E501 F841 - tests/test_utils_deprecate.py F841 E501 + tests/test_utils_defer.py E501 + tests/test_utils_deprecate.py E501 tests/test_utils_http.py E501 E128 tests/test_utils_iterators.py E501 E128 tests/test_utils_log.py E741 @@ -242,7 +240,7 @@ flake8-ignore = tests/test_utils_reqser.py E501 E128 tests/test_utils_request.py E501 E128 tests/test_utils_response.py E501 - tests/test_utils_signal.py E741 F841 + tests/test_utils_signal.py E741 tests/test_utils_sitemap.py E128 E501 tests/test_utils_url.py E501 E501 tests/test_webclient.py E501 E128 E402 diff --git a/tests/pipelines.py b/tests/pipelines.py index cf677cc17..fed2af7d3 100644 --- a/tests/pipelines.py +++ b/tests/pipelines.py @@ -6,7 +6,7 @@ Some pipelines used for testing class ZeroDivisionErrorPipeline: def open_spider(self, spider): - a = 1 / 0 + 1 / 0 def process_item(self, item, spider): return item diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 9151278a5..ecc0cd7af 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -87,7 +87,7 @@ class CrawlerLoggingTestCase(unittest.TestCase): class MySpider(scrapy.Spider): name = 'spider' - crawler = Crawler(MySpider, {}) + Crawler(MySpider, {}) assert get_scrapy_root_handler() is None def test_spider_custom_settings_log_level(self): @@ -240,13 +240,13 @@ class CrawlerRunnerHasSpider(unittest.TestCase): def test_crawler_runner_asyncio_enabled_true(self): if self.reactor_pytest == 'asyncio': - runner = CrawlerRunner(settings={ + CrawlerRunner(settings={ "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", }) else: msg = r"The installed reactor \(.*?\) does not match the requested one \(.*?\)" with self.assertRaisesRegex(Exception, msg): - runner = CrawlerRunner(settings={ + CrawlerRunner(settings={ "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", }) diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py index a169acbe6..5d0a1d0c9 100644 --- a/tests/test_dependencies.py +++ b/tests/test_dependencies.py @@ -6,7 +6,7 @@ class ScrapyUtilsTest(unittest.TestCase): def test_required_openssl_version(self): try: module = import_module('OpenSSL') - except ImportError as ex: + except ImportError: raise unittest.SkipTest("OpenSSL is not available") if hasattr(module, '__version__'): diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index 873a97248..1e716b94a 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -11,8 +11,6 @@ class TelnetExtensionTest(unittest.TestCase): def _get_console_and_portal(self, settings=None): crawler = get_crawler(settings_dict=settings) console = TelnetConsole(crawler) - username = console.username - password = console.password # This function has some side effects we don't need for this test console._get_telnet_vars = lambda: {} diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index e02b0b840..cbc81bc35 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -715,7 +715,6 @@ class FeedExportTest(unittest.TestCase): @defer.inlineCallbacks def test_export_encoding(self): items = [dict({'foo': u'Test\xd6'})] - header = ['foo'] formats = { 'json': u'[{"foo": "Test\\u00d6"}]'.encode('utf-8'), diff --git a/tests/test_item.py b/tests/test_item.py index 4017f6e84..58735cc5c 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -264,12 +264,12 @@ class ItemTest(unittest.TestCase): """Make sure the DictItem deprecation warning is not issued for Item""" with catch_warnings(record=True) as warnings: - item = Item() + Item() self.assertEqual(len(warnings), 0) class SubclassedItem(Item): pass - subclassed_item = SubclassedItem() + SubclassedItem() self.assertEqual(len(warnings), 0) @@ -321,13 +321,13 @@ class DictItemTest(unittest.TestCase): def test_deprecation_warning(self): with catch_warnings(record=True) as warnings: - dict_item = DictItem() + DictItem() self.assertEqual(len(warnings), 1) self.assertEqual(warnings[0].category, ScrapyDeprecationWarning) with catch_warnings(record=True) as warnings: class SubclassedDictItem(DictItem): pass - subclassed_dict_item = SubclassedDictItem() + SubclassedDictItem() self.assertEqual(len(warnings), 1) self.assertEqual(warnings[0].category, ScrapyDeprecationWarning) diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 5018d6802..5ba03ff4c 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -15,7 +15,7 @@ from scrapy.utils.python import to_bytes skip = False try: from PIL import Image -except ImportError as e: +except ImportError: skip = 'Missing Python Imaging Library, install https://pypi.python.org/pypi/Pillow' else: encoders = set(('jpeg_encoder', 'jpeg_decoder')) diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index 41589177a..ca765518b 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -459,7 +459,6 @@ class TestRequestMetaSettingFallback(TestCase): target = 'http://www.example.com' for settings, response_headers, request_meta, policy_class, check_warning in self.params[3:]: - spider = Spider('foo') mw = RefererMiddleware(Settings(settings)) response = Response(origin, headers=response_headers) @@ -511,7 +510,7 @@ class TestSettingsPolicyByName(TestCase): def test_invalid_name(self): settings = Settings({'REFERRER_POLICY': 'some-custom-unknown-policy'}) with self.assertRaises(RuntimeError): - mw = RefererMiddleware(settings) + RefererMiddleware(settings) class TestPolicyHeaderPredecence001(MixinUnsafeUrl, TestRefererMiddleware): diff --git a/tests/test_utils_defer.py b/tests/test_utils_defer.py index a3b6e64f1..2d4b88121 100644 --- a/tests/test_utils_defer.py +++ b/tests/test_utils_defer.py @@ -64,7 +64,7 @@ class DeferUtilsTest(unittest.TestCase): gotexc = False try: yield process_chain([cb1, cb_fail, cb3], 'res', 'v1', 'v2') - except TypeError as e: + except TypeError: gotexc = True self.assertTrue(gotexc) @@ -104,7 +104,7 @@ class IterErrbackTest(unittest.TestCase): def iterbad(): for x in range(10): if x == 5: - a = 1 / 0 + 1 / 0 yield x errors = [] diff --git a/tests/test_utils_deprecate.py b/tests/test_utils_deprecate.py index adef66c1d..35d35b45d 100644 --- a/tests/test_utils_deprecate.py +++ b/tests/test_utils_deprecate.py @@ -25,7 +25,7 @@ class WarnWhenSubclassedTest(unittest.TestCase): def test_no_warning_on_definition(self): with warnings.catch_warnings(record=True) as w: - Deprecated = create_deprecated_class('Deprecated', NewName) + create_deprecated_class('Deprecated', NewName) w = self._mywarnings(w) self.assertEqual(w, []) @@ -217,7 +217,7 @@ class WarnWhenSubclassedTest(unittest.TestCase): def test_deprecate_a_class_with_custom_metaclass(self): Meta1 = type('Meta1', (type,), {}) New = Meta1('New', (), {}) - Deprecated = create_deprecated_class('Deprecated', New) + create_deprecated_class('Deprecated', New) def test_deprecate_subclass_of_deprecated_class(self): with warnings.catch_warnings(record=True) as w: diff --git a/tests/test_utils_signal.py b/tests/test_utils_signal.py index bb211dc60..c83c9398c 100644 --- a/tests/test_utils_signal.py +++ b/tests/test_utils_signal.py @@ -44,7 +44,7 @@ class SendCatchLogTest(unittest.TestCase): def error_handler(self, arg, handlers_called): handlers_called.add(self.error_handler) - a = 1 / 0 + 1 / 0 def ok_handler(self, arg, handlers_called): handlers_called.add(self.ok_handler) From cb8140a42a27ede87b0880372024f2f1804618b8 Mon Sep 17 00:00:00 2001 From: nsirletti <40069643+nsirletti@users.noreply.github.com> Date: Mon, 11 May 2020 20:20:31 +0200 Subject: [PATCH 2/5] Deprecate Response.body_as_unicode() (#4555) Co-authored-by: Nicolas Sirletti --- docs/topics/request-response.rst | 5 ----- scrapy/http/response/text.py | 5 +++++ tests/test_http_response.py | 9 +++++++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 024f46466..15a83f453 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -834,11 +834,6 @@ TextResponse objects .. automethod:: TextResponse.follow_all - .. method:: TextResponse.body_as_unicode() - - The same as :attr:`text`, but available as a method. This method is - kept for backward compatibility; please prefer ``response.text``. - HtmlResponse objects -------------------- diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 2f0f3820c..5614e6e55 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -5,6 +5,7 @@ discovering (through HTTP headers) to base Response class. See documentation in docs/topics/request-response.rst """ +import warnings from contextlib import suppress from typing import Generator from urllib.parse import urljoin @@ -14,6 +15,7 @@ from w3lib.encoding import (html_body_declared_encoding, html_to_unicode, http_content_type_encoding, resolve_encoding) from w3lib.html import strip_html5_whitespace +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request from scrapy.http.response import Response from scrapy.utils.python import memoizemethod_noargs, to_unicode @@ -61,6 +63,9 @@ class TextResponse(Response): def body_as_unicode(self): """Return body as unicode""" + warnings.warn('Response.body_as_unicode() is deprecated, ' + 'please use Response.text instead.', + ScrapyDeprecationWarning) return self.text @property diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 43d6d936a..2f73afe56 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -1,7 +1,9 @@ import unittest +from warnings import catch_warnings from w3lib.encoding import resolve_encoding +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import (Request, Response, TextResponse, HtmlResponse, XmlResponse, Headers) from scrapy.selector import Selector @@ -660,6 +662,13 @@ class TextResponseTest(BaseResponseTest): with self.assertRaises(ValueError): response.follow_all(css='a[href*="example.com"]', xpath='//a[contains(@href, "example.com")]') + def test_body_as_unicode_deprecation_warning(self): + with catch_warnings(record=True) as warnings: + r1 = self.response_class("http://www.example.com", body=u'Hello', encoding='utf-8') + self.assertEqual(r1.body_as_unicode(), u'Hello') + self.assertEqual(len(warnings), 1) + self.assertEqual(warnings[0].category, ScrapyDeprecationWarning) + class HtmlResponseTest(TextResponseTest): From cf9be5344a89dd8e14f8241ec69de9c984ec1e05 Mon Sep 17 00:00:00 2001 From: willbeaufoy Date: Mon, 11 May 2020 19:35:25 +0100 Subject: [PATCH 3/5] Prevent create_instance() returning None (#4532) Currently create_instance() can return None if an extension is incorrectly implemented, but the extension will still show up as enabled in the logs. This can cause confusion, as in the linked bug. This change prevents this occurring by throwing an error if create_instance() will return None. --- scrapy/utils/misc.py | 15 ++++++++++++--- tests/test_utils_misc/__init__.py | 14 +++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index 52cfba208..ab7cf9deb 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -137,17 +137,26 @@ def create_instance(objcls, settings, crawler, *args, **kwargs): ``*args`` and ``**kwargs`` are forwarded to the constructors. Raises ``ValueError`` if both ``settings`` and ``crawler`` are ``None``. + + Raises ``TypeError`` if the resulting instance is ``None`` (e.g. if an + extension has not been implemented correctly). """ if settings is None: if crawler is None: raise ValueError("Specify at least one of settings and crawler.") settings = crawler.settings if crawler and hasattr(objcls, 'from_crawler'): - return objcls.from_crawler(crawler, *args, **kwargs) + instance = objcls.from_crawler(crawler, *args, **kwargs) + method_name = 'from_crawler' elif hasattr(objcls, 'from_settings'): - return objcls.from_settings(settings, *args, **kwargs) + instance = objcls.from_settings(settings, *args, **kwargs) + method_name = 'from_settings' else: - return objcls(*args, **kwargs) + instance = objcls(*args, **kwargs) + method_name = '__new__' + if instance is None: + raise TypeError("%s.%s returned None" % (objcls.__qualname__, method_name)) + return instance @contextmanager diff --git a/tests/test_utils_misc/__init__.py b/tests/test_utils_misc/__init__.py index 6f945cd01..015a0e5a2 100644 --- a/tests/test_utils_misc/__init__.py +++ b/tests/test_utils_misc/__init__.py @@ -114,8 +114,12 @@ class UtilsMiscTestCase(unittest.TestCase): # 2. with from_settings() constructor # 3. with from_crawler() constructor # 4. with from_settings() and from_crawler() constructor - spec_sets = ([], ['from_settings'], ['from_crawler'], - ['from_settings', 'from_crawler']) + spec_sets = ( + ['__qualname__'], + ['__qualname__', 'from_settings'], + ['__qualname__', 'from_crawler'], + ['__qualname__', 'from_settings', 'from_crawler'], + ) for specs in spec_sets: m = mock.MagicMock(spec_set=specs) _test_with_settings(m, settings) @@ -123,7 +127,7 @@ class UtilsMiscTestCase(unittest.TestCase): _test_with_crawler(m, settings, crawler) # Check adoption of crawler settings - m = mock.MagicMock(spec_set=['from_settings']) + m = mock.MagicMock(spec_set=['__qualname__', 'from_settings']) create_instance(m, None, crawler, *args, **kwargs) m.from_settings.assert_called_once_with(crawler.settings, *args, **kwargs) @@ -131,6 +135,10 @@ class UtilsMiscTestCase(unittest.TestCase): with self.assertRaises(ValueError): create_instance(m, None, None) + m.from_settings.return_value = None + with self.assertRaises(TypeError): + create_instance(m, settings, None) + def test_set_environ(self): assert os.environ.get('some_test_environ') is None with set_environ(some_test_environ='test_value'): From 97532a9144cc011e5a6ede84fa7d317c3af60131 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Tue, 12 May 2020 20:40:09 +0530 Subject: [PATCH 4/5] test(spiderloader): no duplicate spider names (#4560) --- tests/test_spiderloader/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index d8be6e277..b6fb27ffe 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -137,6 +137,11 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): msg = str(w[0].message) self.assertIn("several spiders with the same name", msg) self.assertIn("'spider3'", msg) + self.assertTrue(msg.count("'spider3'") == 2) + + self.assertNotIn("'spider1'", msg) + self.assertNotIn("'spider2'", msg) + self.assertNotIn("'spider4'", msg) spiders = set(spider_loader.list()) self.assertEqual(spiders, set(['spider1', 'spider2', 'spider3', 'spider4'])) @@ -156,7 +161,13 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): msg = str(w[0].message) self.assertIn("several spiders with the same name", msg) self.assertIn("'spider1'", msg) + self.assertTrue(msg.count("'spider1'") == 2) + self.assertIn("'spider2'", msg) + self.assertTrue(msg.count("'spider2'") == 2) + + self.assertNotIn("'spider3'", msg) + self.assertNotIn("'spider4'", msg) spiders = set(spider_loader.list()) self.assertEqual(spiders, set(['spider1', 'spider2', 'spider3', 'spider4'])) From 8d1269bcbc81fa0bb5a69068e07bdbcb0dba8889 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Wed, 13 May 2020 00:12:28 +0530 Subject: [PATCH 5/5] Cover chompjs in documentation (#4562) --- docs/topics/dynamic-content.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/topics/dynamic-content.rst b/docs/topics/dynamic-content.rst index 3b85bfe8a..495111b56 100644 --- a/docs/topics/dynamic-content.rst +++ b/docs/topics/dynamic-content.rst @@ -184,6 +184,18 @@ data from it: >>> json.loads(json_data) {'field': 'value'} +- chompjs_ provides an API to parse JavaScript objects into a :class:`dict`. + + For example, if the JavaScript code contains + ``var data = {field: "value", secondField: "second value"};`` + you can extract that data as follows: + + >>> import chompjs + >>> javascript = response.css('script::text').get() + >>> data = chompjs.parse_js_object(javascript) + >>> data + {'field': 'value', 'secondField': 'second value'} + - Otherwise, use js2xml_ to convert the JavaScript code into an XML document that you can parse using :ref:`selectors `. @@ -241,6 +253,7 @@ along with `scrapy-selenium`_ for seamless integration. .. _AJAX: https://en.wikipedia.org/wiki/Ajax_%28programming%29 +.. _chompjs: https://github.com/Nykakin/chompjs .. _CSS: https://en.wikipedia.org/wiki/Cascading_Style_Sheets .. _curl: https://curl.haxx.se/ .. _headless browser: https://en.wikipedia.org/wiki/Headless_browser