Repository: scrapy/scrapy
Pull request #4440 (2020-03-18 03:47:42)

Remove assertions from production code
Inspired by the following comment by @Gallaecio: https://github.com/scrapy/scrapy/pull/4054#discussion_r351514071

From [the docs about the `assert` statement](https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement):
> The current code generator emits no code for an assert statement when optimization is requested at compile time

Please pay attention to the specific exception classes, there might be better ones in some cases. 

codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/4440?src=pr&el=h1) Report
> Merging [#4440](https://codecov.io/gh/scrapy/scrapy/pull/4440?src=pr&el=desc) into [master](https://codecov.io/gh/scrapy/scrapy/commit/533131a30fa944688dc54dd82a581739d1ed247c?src=pr&el=desc) will **decrease** coverage by `0.25%`.
> The diff coverage is `11.76%`.


```diff
@@            Coverage Diff             @@
##           master    #4440      +/-   ##
==========================================
- Coverage   84.78%   84.52%   -0.26%     
==========================================
  Files         164      164              
  Lines        9883     9900      +17     
  Branches     1469     1485      +16     
==========================================
- Hits         8379     8368      -11     
- Misses       1248     1263      +15     
- Partials      256      269      +13
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/4440?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/contracts/default.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2NvbnRyYWN0cy9kZWZhdWx0LnB5) | `84.31% <0%> (-3.69%)` | :arrow_down: |
| [scrapy/pipelines/files.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L3BpcGVsaW5lcy9maWxlcy5weQ==) | `60.92% <0%> (-0.74%)` | :arrow_down: |
| [scrapy/core/scraper.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvc2NyYXBlci5weQ==) | `87.97% <0%> (-1.2%)` | :arrow_down: |
| [scrapy/core/engine.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZW5naW5lLnB5) | `87.87% <0%> (-5.02%)` | :arrow_down: |
| [scrapy/crawler.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2NyYXdsZXIucHk=) | `87.86% <0%> (-1.1%)` | :arrow_down: |
| [scrapy/http/request/\_\_init\_\_.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2h0dHAvcmVxdWVzdC9fX2luaXRfXy5weQ==) | `97.26% <0%> (-2.74%)` | :arrow_down: |
| [scrapy/commands/\_\_init\_\_.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2NvbW1hbmRzL19faW5pdF9fLnB5) | `61.11% <0%> (-1.16%)` | :arrow_down: |
| [scrapy/utils/reactor.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL3JlYWN0b3IucHk=) | `80% <0%> (-3.34%)` | :arrow_down: |
| [scrapy/utils/iterators.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL2l0ZXJhdG9ycy5weQ==) | `97.75% <100%> (+0.05%)` | :arrow_up: |
| [scrapy/core/downloader/middleware.py](https://codecov.io/gh/scrapy/scrapy/pull/4440/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci9taWRkbGV3YXJlLnB5) | `96.36% <33.33%> (-3.64%)` | :arrow_down: |

elacuesta: I wonder why there is no Travis build for the latest commit :thinking: 
Gallaecio: [There is](https://travis-ci.org/github/scrapy/scrapy/builds/668922915). No idea why it does not show up in the pull request any more, though.

Patch:
diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst
index e3e8fdc72..cf1de1bd1 100644
--- a/docs/topics/practices.rst
+++ b/docs/topics/practices.rst
@@ -35,8 +35,9 @@ Here's an example showing how to run a single spider with it.
         ...
 
     process = CrawlerProcess(settings={
-        'FEED_FORMAT': 'json',
-        'FEED_URI': 'items.json'
+        "FEEDS": {
+            "items.json": {"format": "json"},
+        },
     })
 
     process.crawl(MySpider)
diff --git a/docs/topics/shell.rst b/docs/topics/shell.rst
index 8f7518b19..0f46f1c87 100644
--- a/docs/topics/shell.rst
+++ b/docs/topics/shell.rst
@@ -156,6 +156,17 @@ First, we launch the shell::
 
     scrapy shell 'https://scrapy.org' --nolog
 
+.. note::
+
+   Remember to always enclose URLs in quotes when running the Scrapy shell from
+   the command line, otherwise URLs containing arguments (i.e. the ``&`` character)
+   will not work.
+
+   On Windows, use double quotes instead::
+
+       scrapy shell "https://scrapy.org" --nolog
+
+
 Then, the shell fetches the URL (using the Scrapy downloader) and prints the
 list of available objects and useful shortcuts (you'll notice that these lines
 all start with the ``[s]`` prefix)::
diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst
index 2def53848..8661f86a0 100644
--- a/docs/topics/signals.rst
+++ b/docs/topics/signals.rst
@@ -16,8 +16,7 @@ deliver the arguments that the handler receives.
 You can connect to signals (or send your own) through the
 :ref:`topics-api-signals`.
 
-Here is a simple example showing how you can catch signals and perform some action:
-::
+Here is a simple example showing how you can catch signals and perform some action::
 
     from scrapy import signals
     from scrapy import Spider
@@ -52,9 +51,45 @@ Deferred signal handlers
 ========================
 
 Some signals support returning :class:`~twisted.internet.defer.Deferred`
-objects from their handlers, see the :ref:`topics-signals-ref` below to know
-which ones.
+objects from their handlers, allowing you to run asynchronous code that
+does not block Scrapy. If a signal handler returns a
+:class:`~twisted.internet.defer.Deferred`, Scrapy waits for that
+:class:`~twisted.internet.defer.Deferred` to fire.
 
+Let's take an example::
+
+    class SignalSpider(scrapy.Spider):
+        name = 'signals'
+        start_urls = ['http://quotes.toscrape.com/page/1/']
+
+        @classmethod
+        def from_crawler(cls, crawler, *args, **kwargs):
+            spider = super(SignalSpider, cls).from_crawler(crawler, *args, **kwargs)
+            crawler.signals.connect(spider.item_scraped, signal=signals.item_scraped)
+            return spider
+
+        def item_scraped(self, item):
+            # Send the scraped item to the server
+            d = treq.post(
+                'http://example.com/post',
+                json.dumps(item).encode('ascii'),
+                headers={b'Content-Type': [b'application/json']}
+            )
+
+            # The next item will be scraped only after
+            # deferred (d) is fired
+            return d
+
+        def parse(self, response):
+            for quote in response.css('div.quote'):
+                yield {
+                    'text': quote.css('span.text::text').get(),
+                    'author': quote.css('small.author::text').get(),
+                    'tags': quote.css('div.tags a.tag::text').getall(),
+                }
+
+See the :ref:`topics-signals-ref` below to know which signals support
+:class:`~twisted.internet.defer.Deferred`.
 
 .. _topics-signals-ref:
 
@@ -66,9 +101,12 @@ Built-in signals reference
 
 Here's the list of Scrapy built-in signals and their meaning.
 
-engine_started
+Engine signals
 --------------
 
+engine_started
+~~~~~~~~~~~~~~
+
 .. signal:: engine_started
 .. function:: engine_started()
 
@@ -81,7 +119,7 @@ engine_started
     getting fired before :signal:`spider_opened`.
 
 engine_stopped
---------------
+~~~~~~~~~~~~~~
 
 .. signal:: engine_stopped
 .. function:: engine_stopped()
@@ -91,9 +129,20 @@ engine_stopped
 
     This signal supports returning deferreds from their handlers.
 
-item_scraped
+Item signals
 ------------
 
+.. note::
+    As at max :setting:`CONCURRENT_ITEMS` items are processed in
+    parallel, many deferreds are fired together using
+    :class:`~twisted.internet.defer.DeferredList`. Hence the next
+    batch waits for the :class:`~twisted.internet.defer.DeferredList`
+    to fire and then runs the respective item signal handler for
+    the next batch of scraped items.
+
+item_scraped
+~~~~~~~~~~~~
+
 .. signal:: item_scraped
 .. function:: item_scraped(item, response, spider)
 
@@ -112,7 +161,7 @@ item_scraped
     :type response: :class:`~scrapy.http.Response` object
 
 item_dropped
-------------
+~~~~~~~~~~~~
 
 .. signal:: item_dropped
 .. function:: item_dropped(item, response, exception, spider)
@@ -137,7 +186,7 @@ item_dropped
     :type exception: :exc:`~scrapy.exceptions.DropItem` exception
 
 item_error
-------------
+~~~~~~~~~~
 
 .. signal:: item_error
 .. function:: item_error(item, response, spider, failure)
@@ -159,8 +208,11 @@ item_error
     :param failure: the exception raised
     :type failure: twisted.python.failure.Failure
 
+Spider signals
+--------------
+
 spider_closed
--------------
+~~~~~~~~~~~~~
 
 .. signal:: spider_closed
 .. function:: spider_closed(spider, reason)
@@ -183,7 +235,7 @@ spider_closed
     :type reason: str
 
 spider_opened
--------------
+~~~~~~~~~~~~~
 
 .. signal:: spider_opened
 .. function:: spider_opened(spider)
@@ -198,7 +250,7 @@ spider_opened
     :type spider: :class:`~scrapy.spiders.Spider` object
 
 spider_idle
------------
+~~~~~~~~~~~
 
 .. signal:: spider_idle
 .. function:: spider_idle(spider)
@@ -228,7 +280,7 @@ spider_idle
     due to duplication).
 
 spider_error
-------------
+~~~~~~~~~~~~
 
 .. signal:: spider_error
 .. function:: spider_error(failure, response, spider)
@@ -246,8 +298,11 @@ spider_error
     :param spider: the spider which raised the exception
     :type spider: :class:`~scrapy.spiders.Spider` object
 
+Request signals
+---------------
+
 request_scheduled
------------------
+~~~~~~~~~~~~~~~~~
 
 .. signal:: request_scheduled
 .. function:: request_scheduled(request, spider)
@@ -264,7 +319,7 @@ request_scheduled
     :type spider: :class:`~scrapy.spiders.Spider` object
 
 request_dropped
----------------
+~~~~~~~~~~~~~~~
 
 .. signal:: request_dropped
 .. function:: request_dropped(request, spider)
@@ -281,7 +336,7 @@ request_dropped
     :type spider: :class:`~scrapy.spiders.Spider` object
 
 request_reached_downloader
----------------------------
+~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 .. signal:: request_reached_downloader
 .. function:: request_reached_downloader(request, spider)
@@ -297,7 +352,7 @@ request_reached_downloader
     :type spider: :class:`~scrapy.spiders.Spider` object
 
 request_left_downloader
------------------------
+~~~~~~~~~~~~~~~~~~~~~~~
 
 .. signal:: request_left_downloader
 .. function:: request_left_downloader(request, spider)
@@ -315,8 +370,11 @@ request_left_downloader
     :param spider: the spider that yielded the request
     :type spider: :class:`~scrapy.spiders.Spider` object
 
+Response signals
+----------------
+
 response_received
------------------
+~~~~~~~~~~~~~~~~~
 
 .. signal:: response_received
 .. function:: response_received(response, request, spider)
@@ -336,7 +394,7 @@ response_received
     :type spider: :class:`~scrapy.spiders.Spider` object
 
 response_downloaded
--------------------
+~~~~~~~~~~~~~~~~~~~
 
 .. signal:: response_downloaded
 .. function:: response_downloaded(response, request, spider)
diff --git a/scrapy/commands/__init__.py b/scrapy/commands/__init__.py
index a573a03d9..9f8e6986a 100644
--- a/scrapy/commands/__init__.py
+++ b/scrapy/commands/__init__.py
@@ -23,7 +23,8 @@ class ScrapyCommand:
         self.settings = None  # set in scrapy.cmdline
 
     def set_crawler(self, crawler):
-        assert not hasattr(self, '_crawler'), "crawler already set"
+        if hasattr(self, '_crawler'):
+            raise RuntimeError("crawler already set")
         self._crawler = crawler
 
     def syntax(self):
diff --git a/scrapy/contracts/default.py b/scrapy/contracts/default.py
index 3002fc702..a1b0f8f22 100644
--- a/scrapy/contracts/default.py
+++ b/scrapy/contracts/default.py
@@ -58,7 +58,11 @@ class ReturnsContract(Contract):
     def __init__(self, *args, **kwargs):
         super(ReturnsContract, self).__init__(*args, **kwargs)
 
-        assert len(self.args) in [1, 2, 3]
+        if len(self.args) not in [1, 2, 3]:
+            raise ValueError(
+                "Incorrect argument quantity: expected 1, 2 or 3, got %i"
+                % len(self.args)
+            )
         self.obj_name = self.args[0] or None
         self.obj_type = self.objects[self.obj_name]
 
diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py
index 5a03dcdf7..4c2eea522 100644
--- a/scrapy/core/downloader/middleware.py
+++ b/scrapy/core/downloader/middleware.py
@@ -45,8 +45,9 @@ class DownloaderMiddlewareManager(MiddlewareManager):
 
         @defer.inlineCallbacks
         def process_response(response):
-            assert response is not None, 'Received None in process_response'
-            if isinstance(response, Request):
+            if response is None:
+                raise TypeError("Received None in process_response")
+            elif isinstance(response, Request):
                 return response
 
             for method in self.methods['process_response']:
diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py
index 66cf9ad9a..77d71846e 100644
--- a/scrapy/core/engine.py
+++ b/scrapy/core/engine.py
@@ -73,7 +73,8 @@ class ExecutionEngine:
     @defer.inlineCallbacks
     def start(self):
         """Start the execution engine"""
-        assert not self.running, "Engine already running"
+        if self.running:
+            raise RuntimeError("Engine already running")
         self.start_time = time()
         yield self.signals.send_catch_log_deferred(signal=signals.engine_started)
         self.running = True
@@ -82,7 +83,8 @@ class ExecutionEngine:
 
     def stop(self):
         """Stop the execution engine gracefully"""
-        assert self.running, "Engine not running"
+        if not self.running:
+            raise RuntimeError("Engine not running")
         self.running = False
         dfd = self._close_all_spiders()
         return dfd.addBoth(lambda _: self._finish_stopping_engine())
@@ -165,7 +167,11 @@ class ExecutionEngine:
         return d
 
     def _handle_downloader_output(self, response, request, spider):
-        assert isinstance(response, (Request, Response, Failure)), response
+        if not isinstance(response, (Request, Response, Failure)):
+            raise TypeError(
+                "Incorrect type: expected Request, Response or Failure, got %s: %r"
+                % (type(response), response)
+            )
         # downloader middleware can return requests (for example, redirects)
         if isinstance(response, Request):
             self.crawl(response, spider)
@@ -205,8 +211,8 @@ class ExecutionEngine:
         return not bool(self.slot)
 
     def crawl(self, request, spider):
-        assert spider in self.open_spiders, \
-            "Spider %r not opened when crawling: %s" % (spider.name, request)
+        if spider not in self.open_spiders:
+            raise RuntimeError("Spider %r not opened when crawling: %s" % (spider.name, request))
         self.schedule(request, spider)
         self.slot.nextcall.schedule()
 
@@ -232,7 +238,11 @@ class ExecutionEngine:
         slot.add_request(request)
 
         def _on_success(response):
-            assert isinstance(response, (Response, Request))
+            if not isinstance(response, (Response, Request)):
+                raise TypeError(
+                    "Incorrect type: expected Response or Request, got %s: %r"
+                    % (type(response), response)
+                )
             if isinstance(response, Response):
                 response.request = request  # tie request to response received
                 logkws = self.logformatter.crawled(request, response, spider)
@@ -253,8 +263,8 @@ class ExecutionEngine:
 
     @defer.inlineCallbacks
     def open_spider(self, spider, start_requests=(), close_if_idle=True):
-        assert self.has_capacity(), "No free spider slot when opening %r" % \
-            spider.name
+        if not self.has_capacity():
+            raise RuntimeError("No free spider slot when opening %r" % spider.name)
         logger.info("Spider opened", extra={'spider': spider})
         nextcall = CallLaterOnce(self._next_request, spider)
         scheduler = self.scheduler_cls.from_crawler(self.crawler)
diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py
index 3e4826216..edbb4dd66 100644
--- a/scrapy/core/scraper.py
+++ b/scrapy/core/scraper.py
@@ -123,7 +123,11 @@ class Scraper:
     def _scrape(self, response, request, spider):
         """Handle the downloaded response or failure through the spider
         callback/errback"""
-        assert isinstance(response, (Response, Failure))
+        if not isinstance(response, (Response, Failure)):
+            raise TypeError(
+                "Incorrect type: expected Response or Failure, got %s: %r"
+                % (type(response), response)
+            )
 
         dfd = self._scrape2(response, request, spider)  # returns spider's processed output
         dfd.addErrback(self.handle_spider_error, request, response, spider)
diff --git a/scrapy/crawler.py b/scrapy/crawler.py
index 20990ea41..6f43771e2 100644
--- a/scrapy/crawler.py
+++ b/scrapy/crawler.py
@@ -78,7 +78,8 @@ class Crawler:
 
     @defer.inlineCallbacks
     def crawl(self, *args, **kwargs):
-        assert not self.crawling, "Crawling already taking place"
+        if self.crawling:
+            raise RuntimeError("Crawling already taking place")
         self.crawling = True
 
         try:
diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py
index 0a6637af8..a98ba9960 100644
--- a/scrapy/http/request/__init__.py
+++ b/scrapy/http/request/__init__.py
@@ -24,7 +24,8 @@ class Request(object_ref):
         self.method = str(method).upper()
         self._set_url(url)
         self._set_body(body)
-        assert isinstance(priority, int), "Request priority not an integer: %r" % priority
+        if not isinstance(priority, int):
+            raise TypeError("Request priority not an integer: %r" % priority)
         self.priority = priority
 
         if callback is not None and not callable(callback):
diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index aab645d3d..ae365db5b 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -106,7 +106,8 @@ class S3FilesStore:
         else:
             from boto.s3.connection import S3Connection
             self.S3Connection = S3Connection
-        assert uri.startswith('s3://')
+        if not uri.startswith("s3://"):
+            raise ValueError("Incorrect URI scheme in %s, expected 's3'" % uri)
         self.bucket, self.prefix = uri[5:].split('/', 1)
 
     def stat_file(self, path, info):
@@ -266,7 +267,8 @@ class FTPFilesStore:
     USE_ACTIVE_MODE = None
 
     def __init__(self, uri):
-        assert uri.startswith('ftp://')
+        if not uri.startswith("ftp://"):
+            raise ValueError("Incorrect URI scheme in %s, expected 'ftp'" % uri)
         u = urlparse(uri)
         self.port = u.port
         self.host = u.hostname
diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py
index b71419111..5e15bf0c8 100644
--- a/scrapy/utils/iterators.py
+++ b/scrapy/utils/iterators.py
@@ -128,10 +128,12 @@ def csviter(obj, delimiter=None, headers=None, encoding=None, quotechar=None):
 
 def _body_or_str(obj, unicode=True):
     expected_types = (Response, str, bytes)
-    assert isinstance(obj, expected_types), \
-        "obj must be %s, not %s" % (
-            " or ".join(t.__name__ for t in expected_types),
-            type(obj).__name__)
+    if not isinstance(obj, expected_types):
+        expected_types_str = " or ".join(t.__name__ for t in expected_types)
+        raise TypeError(
+            "Object %r must be %s, not %s"
+            % (obj, expected_types_str, type(obj).__name__)
+        )
     if isinstance(obj, Response):
         if not unicode:
             return obj.body
diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py
index 5308812d6..3c705f69b 100644
--- a/scrapy/utils/reactor.py
+++ b/scrapy/utils/reactor.py
@@ -9,7 +9,8 @@ from scrapy.utils.misc import load_object
 def listen_tcp(portrange, host, factory):
     """Like reactor.listenTCP but tries different ports in a range."""
     from twisted.internet import reactor
-    assert len(portrange) <= 2, "invalid portrange: %s" % portrange
+    if len(portrange) > 2:
+        raise ValueError("invalid portrange: %s" % portrange)
     if not portrange:
         return reactor.listenTCP(0, factory, interface=host)
     if not hasattr(portrange, '__iter__'):
diff --git a/scrapy/utils/reqser.py b/scrapy/utils/reqser.py
index 749bbc387..5ea2aafb8 100644
--- a/scrapy/utils/reqser.py
+++ b/scrapy/utils/reqser.py
@@ -1,6 +1,8 @@
 """
 Helper functions for serializing (and deserializing) requests.
 """
+import inspect
+
 from scrapy.http import Request
 from scrapy.utils.python import to_unicode
 from scrapy.utils.misc import load_object
@@ -68,20 +70,6 @@ def request_from_dict(d, spider=None):
     )
 
 
-def _is_private_method(name):
-    return name.startswith('__') and not name.endswith('__')
-
-
-def _mangle_private_name(obj, func, name):
-    qualname = getattr(func, '__qualname__', None)
-    if qualname is None:
-        classname = obj.__class__.__name__.lstrip('_')
-        return '_%s%s' % (classname, name)
-    else:
-        splits = qualname.split('.')
-        return '_%s%s' % (splits[-2], splits[-1])
-
-
 def _find_method(obj, func):
     if obj:
         try:
@@ -90,10 +78,17 @@ def _find_method(obj, func):
             pass
         else:
             if func_self is obj:
-                name = func.__func__.__name__
-                if _is_private_method(name):
-                    return _mangle_private_name(obj, func, name)
-                return name
+                members = inspect.getmembers(obj, predicate=inspect.ismethod)
+                for name, obj_func in members:
+                    # We need to use __func__ to access the original
+                    # function object because instance method objects
+                    # are generated each time attribute is retrieved from
+                    # instance.
+                    #
+                    # Reference: The standard type hierarchy
+                    # https://docs.python.org/3/reference/datamodel.html
+                    if obj_func.__func__ is func.__func__:
+                        return name
     raise ValueError("Function %s is not a method of: %s" % (func, obj))
 
 


Test patch:
diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py
index ec8311298..a85087619 100644
--- a/tests/test_utils_iterators.py
+++ b/tests/test_utils_iterators.py
@@ -157,7 +157,7 @@ class XmliterTestCase(unittest.TestCase):
 
     def test_xmliter_objtype_exception(self):
         i = self.xmliter(42, 'product')
-        self.assertRaises(AssertionError, next, i)
+        self.assertRaises(TypeError, next, i)
 
     def test_xmliter_encoding(self):
         body = b'<?xml version="1.0" encoding="ISO-8859-9"?>\n<xml>\n    <item>Some Turkish Characters \xd6\xc7\xde\xdd\xd0\xdc \xfc\xf0\xfd\xfe\xe7\xf6</item>\n</xml>\n\n'
diff --git a/tests/test_utils_reqser.py b/tests/test_utils_reqser.py
index c7572f02c..50b026d1c 100644
--- a/tests/test_utils_reqser.py
+++ b/tests/test_utils_reqser.py
@@ -2,7 +2,7 @@ import unittest
 
 from scrapy.http import Request, FormRequest
 from scrapy.spiders import Spider
-from scrapy.utils.reqser import request_to_dict, request_from_dict, _is_private_method, _mangle_private_name
+from scrapy.utils.reqser import request_to_dict, request_from_dict
 
 
 class RequestSerializationTest(unittest.TestCase):
@@ -69,6 +69,26 @@ class RequestSerializationTest(unittest.TestCase):
                     errback=self.spider.handle_error)
         self._assert_serializes_ok(r, spider=self.spider)
 
+    def test_reference_callback_serialization(self):
+        r = Request("http://www.example.com",
+                    callback=self.spider.parse_item_reference,
+                    errback=self.spider.handle_error_reference)
+        self._assert_serializes_ok(r, spider=self.spider)
+        request_dict = request_to_dict(r, self.spider)
+        self.assertEqual(request_dict['callback'], 'parse_item_reference')
+        self.assertEqual(request_dict['errback'], 'handle_error_reference')
+
+    def test_private_reference_callback_serialization(self):
+        r = Request("http://www.example.com",
+                    callback=self.spider._TestSpider__parse_item_reference,
+                    errback=self.spider._TestSpider__handle_error_reference)
+        self._assert_serializes_ok(r, spider=self.spider)
+        request_dict = request_to_dict(r, self.spider)
+        self.assertEqual(request_dict['callback'],
+                         '_TestSpider__parse_item_reference')
+        self.assertEqual(request_dict['errback'],
+                         '_TestSpider__handle_error_reference')
+
     def test_private_callback_serialization(self):
         r = Request("http://www.example.com",
                     callback=self.spider._TestSpider__parse_item_private,
@@ -81,41 +101,6 @@ class RequestSerializationTest(unittest.TestCase):
                     errback=self.spider.handle_error)
         self._assert_serializes_ok(r, spider=self.spider)
 
-    def test_private_callback_name_matching(self):
-        self.assertTrue(_is_private_method('__a'))
-        self.assertTrue(_is_private_method('__a_'))
-        self.assertTrue(_is_private_method('__a_a'))
-        self.assertTrue(_is_private_method('__a_a_'))
-        self.assertTrue(_is_private_method('__a__a'))
-        self.assertTrue(_is_private_method('__a__a_'))
-        self.assertTrue(_is_private_method('__a___a'))
-        self.assertTrue(_is_private_method('__a___a_'))
-        self.assertTrue(_is_private_method('___a'))
-        self.assertTrue(_is_private_method('___a_'))
-        self.assertTrue(_is_private_method('___a_a'))
-        self.assertTrue(_is_private_method('___a_a_'))
-        self.assertTrue(_is_private_method('____a_a_'))
-
-        self.assertFalse(_is_private_method('_a'))
-        self.assertFalse(_is_private_method('_a_'))
-        self.assertFalse(_is_private_method('__a__'))
-        self.assertFalse(_is_private_method('__'))
-        self.assertFalse(_is_private_method('___'))
-        self.assertFalse(_is_private_method('____'))
-
-    def _assert_mangles_to(self, obj, name):
-        func = getattr(obj, name)
-        self.assertEqual(
-            _mangle_private_name(obj, func, func.__name__),
-            name
-        )
-
-    def test_private_name_mangling(self):
-        self._assert_mangles_to(
-            self.spider, '_TestSpider__parse_item_private')
-        self._assert_mangles_to(
-            self.spider, '_TestSpiderMixin__mixin_callback')
-
     def test_unserializable_callback1(self):
         r = Request("http://www.example.com", callback=lambda x: x)
         self.assertRaises(ValueError, request_to_dict, r)
@@ -125,14 +110,49 @@ class RequestSerializationTest(unittest.TestCase):
         r = Request("http://www.example.com", callback=self.spider.parse_item)
         self.assertRaises(ValueError, request_to_dict, r)
 
+    def test_unserializable_callback3(self):
+        """Parser method is removed or replaced dynamically."""
+
+        class MySpider(Spider):
+
+            name = 'my_spider'
+
+            def parse(self, response):
+                pass
+
+        spider = MySpider()
+        r = Request("http://www.example.com", callback=spider.parse)
+        setattr(spider, 'parse', None)
+        self.assertRaises(ValueError, request_to_dict, r, spider=spider)
+
 
 class TestSpiderMixin:
     def __mixin_callback(self, response):
         pass
 
 
+def parse_item(response):
+    pass
+
+
+def handle_error(failure):
+    pass
+
+
+def private_parse_item(response):
+    pass
+
+
+def private_handle_error(failure):
+    pass
+
+
 class TestSpider(Spider, TestSpiderMixin):
     name = 'test'
+    parse_item_reference = parse_item
+    handle_error_reference = handle_error
+    __parse_item_reference = private_parse_item
+    __handle_error_reference = private_handle_error
 
     def parse_item(self, response):
         pass