Repository: scrapy/scrapy
Pull request #1473 (2015-09-01 18:35:14)

[MRG+1] Support for returning deferreds in middlewares
Adds support for returning deferreds in middlewares, and makes use of this to fix a limitation in RobotsTxtMiddleware.
#1471


Patch:
diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst
index 73cc67423..38c9456db 100644
--- a/docs/topics/downloader-middleware.rst
+++ b/docs/topics/downloader-middleware.rst
@@ -58,6 +58,8 @@ more of the following methods:
 
 .. class:: DownloaderMiddleware
 
+   .. note::  Any of the downloader middleware methods may also return a deferred.
+
    .. method:: process_request(request, spider)
 
       This method is called for each request that goes through the download
@@ -877,12 +879,6 @@ RobotsTxtMiddleware
     To make sure Scrapy respects robots.txt make sure the middleware is enabled
     and the :setting:`ROBOTSTXT_OBEY` setting is enabled.
 
-    .. warning:: Keep in mind that, if you crawl using multiple concurrent
-       requests per domain, Scrapy could still download some forbidden pages
-       if they were requested before the robots.txt file was downloaded. This
-       is a known limitation of the current robots.txt middleware and will
-       be fixed in the future.
-
 .. reqmeta:: dont_obey_robotstxt
 
 If :attr:`Request.meta <scrapy.http.Request.meta>` has
diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py
index 413a05dd1..9cd30c144 100644
--- a/scrapy/core/downloader/middleware.py
+++ b/scrapy/core/downloader/middleware.py
@@ -4,11 +4,15 @@ Downloader Middleware manager
 See documentation in docs/topics/downloader-middleware.rst
 """
 import six
+
+from twisted.internet import defer
+
 from scrapy.http import Request, Response
 from scrapy.middleware import MiddlewareManager
 from scrapy.utils.defer import mustbe_deferred
 from scrapy.utils.conf import build_component_list
 
+
 class DownloaderMiddlewareManager(MiddlewareManager):
 
     component_name = 'downloader middleware'
@@ -27,40 +31,45 @@ class DownloaderMiddlewareManager(MiddlewareManager):
             self.methods['process_exception'].insert(0, mw.process_exception)
 
     def download(self, download_func, request, spider):
+        @defer.inlineCallbacks
         def process_request(request):
             for method in self.methods['process_request']:
-                response = method(request=request, spider=spider)
+                response = yield method(request=request, spider=spider)
                 assert response is None or isinstance(response, (Response, Request)), \
                         'Middleware %s.process_request must return None, Response or Request, got %s' % \
                         (six.get_method_self(method).__class__.__name__, response.__class__.__name__)
                 if response:
-                    return response
-            return download_func(request=request, spider=spider)
+                    defer.returnValue(response)
+            defer.returnValue((yield download_func(request=request,spider=spider)))
 
+        @defer.inlineCallbacks
         def process_response(response):
             assert response is not None, 'Received None in process_response'
             if isinstance(response, Request):
-                return response
+                defer.returnValue(response)
 
             for method in self.methods['process_response']:
-                response = method(request=request, response=response, spider=spider)
+                response = yield method(request=request, response=response,
+                                        spider=spider)
                 assert isinstance(response, (Response, Request)), \
                     'Middleware %s.process_response must return Response or Request, got %s' % \
                     (six.get_method_self(method).__class__.__name__, type(response))
                 if isinstance(response, Request):
-                    return response
-            return response
+                    defer.returnValue(response)
+            defer.returnValue(response)
 
+        @defer.inlineCallbacks
         def process_exception(_failure):
             exception = _failure.value
             for method in self.methods['process_exception']:
-                response = method(request=request, exception=exception, spider=spider)
+                response = yield method(request=request, exception=exception,
+                                        spider=spider)
                 assert response is None or isinstance(response, (Response, Request)), \
                     'Middleware %s.process_exception must return None, Response or Request, got %s' % \
                     (six.get_method_self(method).__class__.__name__, type(response))
                 if response:
-                    return response
-            return _failure
+                    defer.returnValue(response)
+            defer.returnValue(_failure)
 
         deferred = mustbe_deferred(process_request, request)
         deferred.addErrback(process_exception)
diff --git a/scrapy/downloadermiddlewares/httpauth.py b/scrapy/downloadermiddlewares/httpauth.py
index 7aa7a62bc..7854e899a 100644
--- a/scrapy/downloadermiddlewares/httpauth.py
+++ b/scrapy/downloadermiddlewares/httpauth.py
@@ -27,5 +27,5 @@ class HttpAuthMiddleware(object):
 
     def process_request(self, request, spider):
         auth = getattr(self, 'auth', None)
-        if auth and b'Authorization' not in request.headers:
-            request.headers[b'Authorization'] = auth
+        if auth and 'Authorization' not in request.headers:
+            request.headers['Authorization'] = auth
diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py
index 457620d85..c061c2407 100644
--- a/scrapy/downloadermiddlewares/robotstxt.py
+++ b/scrapy/downloadermiddlewares/robotstxt.py
@@ -8,6 +8,7 @@ import logging
 
 from six.moves.urllib import robotparser
 
+from twisted.internet.defer import Deferred, maybeDeferred
 from scrapy.exceptions import NotConfigured, IgnoreRequest
 from scrapy.http import Request
 from scrapy.utils.httpobj import urlparse_cached
@@ -34,17 +35,22 @@ class RobotsTxtMiddleware(object):
     def process_request(self, request, spider):
         if request.meta.get('dont_obey_robotstxt'):
             return
-        rp = self.robot_parser(request, spider)
-        if rp and not rp.can_fetch(self._useragent, request.url):
+        d = maybeDeferred(self.robot_parser, request, spider)
+        d.addCallback(self.process_request_2, request, spider)
+        return d
+
+    def process_request_2(self, rp, request, spider):
+        if rp is not None and not rp.can_fetch(self._useragent, request.url):
             logger.debug("Forbidden by robots.txt: %(request)s",
                          {'request': request}, extra={'spider': spider})
-            raise IgnoreRequest
+            raise IgnoreRequest()
 
     def robot_parser(self, request, spider):
         url = urlparse_cached(request)
         netloc = url.netloc
+
         if netloc not in self._parsers:
-            self._parsers[netloc] = None
+            self._parsers[netloc] = Deferred()
             robotsurl = "%s://%s/robots.txt" % (url.scheme, url.netloc)
             robotsreq = Request(
                 robotsurl,
@@ -52,9 +58,19 @@ class RobotsTxtMiddleware(object):
                 meta={'dont_obey_robotstxt': True}
             )
             dfd = self.crawler.engine.download(robotsreq, spider)
-            dfd.addCallback(self._parse_robots)
+            dfd.addCallback(self._parse_robots, netloc)
             dfd.addErrback(self._logerror, robotsreq, spider)
-        return self._parsers[netloc]
+            dfd.addErrback(self._robots_error, netloc)
+
+        if isinstance(self._parsers[netloc], Deferred):
+            d = Deferred()
+            def cb(result):
+                d.callback(result)
+                return result
+            self._parsers[netloc].addCallback(cb)
+            return d
+        else:
+            return self._parsers[netloc]
 
     def _logerror(self, failure, request, spider):
         if failure.type is not IgnoreRequest:
@@ -62,8 +78,9 @@ class RobotsTxtMiddleware(object):
                          {'request': request, 'f_exception': failure.value},
                          exc_info=failure_to_exc_info(failure),
                          extra={'spider': spider})
+        return failure
 
-    def _parse_robots(self, response):
+    def _parse_robots(self, response, netloc):
         rp = robotparser.RobotFileParser(response.url)
         body = ''
         if hasattr(response, 'body_as_unicode'):
@@ -78,4 +95,10 @@ class RobotsTxtMiddleware(object):
                 # 'disallow all' to 'allow any'.
                 pass
         rp.parse(body.splitlines())
-        self._parsers[urlparse_cached(response).netloc] = rp
+
+        rp_dfd = self._parsers[netloc]
+        self._parsers[netloc] = rp
+        rp_dfd.callback(rp)
+
+    def _robots_error(self, failure, netloc):
+        self._parsers.pop(netloc).callback(None)
diff --git a/scrapy/downloadermiddlewares/useragent.py b/scrapy/downloadermiddlewares/useragent.py
index d24750c69..17793cad7 100644
--- a/scrapy/downloadermiddlewares/useragent.py
+++ b/scrapy/downloadermiddlewares/useragent.py
@@ -20,4 +20,4 @@ class UserAgentMiddleware(object):
 
     def process_request(self, request, spider):
         if self.user_agent:
-            request.headers.setdefault(b'User-Agent', self.user_agent)
+            request.headers.setdefault('User-Agent', self.user_agent)
diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py
index a12a2fd07..1920cefe1 100644
--- a/scrapy/http/request/form.py
+++ b/scrapy/http/request/form.py
@@ -159,7 +159,7 @@ def _get_clickable(clickdata, form):
     # If we don't have clickdata, we just use the first clickable element
     if clickdata is None:
         el = clickables[0]
-        return (el.get('name'), el.get('value') or '')
+        return (el.get('name'), el.get('value'))
 
     # If clickdata is given, we compare it to the clickable elements to find a
     # match. We first look to see if the number is specified in clickdata,
@@ -171,7 +171,7 @@ def _get_clickable(clickdata, form):
         except IndexError:
             pass
         else:
-            return (el.get('name'), el.get('value') or '')
+            return (el.get('name'), el.get('value'))
 
     # We didn't find it, so now we build an XPath expression out of the other
     # arguments, because they can be used as such
@@ -179,7 +179,7 @@ def _get_clickable(clickdata, form):
             u''.join(u'[@%s="%s"]' % c for c in six.iteritems(clickdata))
     el = form.xpath(xpath)
     if len(el) == 1:
-        return (el[0].get('name'), el[0].get('value') or '')
+        return (el[0].get('name'), el[0].get('value'))
     elif len(el) > 1:
         raise ValueError("Multiple elements found (%r) matching the criteria "
                          "in clickdata: %r" % (el, clickdata))
diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index 5d48ac1a7..0075667f5 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -218,7 +218,7 @@ REFERER_ENABLED = True
 
 RETRY_ENABLED = True
 RETRY_TIMES = 2  # initial response + 2 retries = 3 requests
-RETRY_HTTP_CODES = [500, 502, 503, 504, 408]
+RETRY_HTTP_CODES = [500, 502, 503, 504, 400, 408]
 RETRY_PRIORITY_ADJUST = -1
 
 ROBOTSTXT_OBEY = False


Test patch:
diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt
index 95e2181e3..759eeffff 100644
--- a/tests/py3-ignores.txt
+++ b/tests/py3-ignores.txt
@@ -6,12 +6,18 @@ tests/test_exporters.py
 tests/test_linkextractors_deprecated.py
 tests/test_crawl.py
 tests/test_downloader_handlers.py
+tests/test_downloadermiddleware_ajaxcrawlable.py
+tests/test_downloadermiddleware_defaultheaders.py
+tests/test_downloadermiddleware_downloadtimeout.py
+tests/test_downloadermiddleware_httpauth.py
 tests/test_downloadermiddleware_httpcache.py
 tests/test_downloadermiddleware_httpcompression.py
 tests/test_downloadermiddleware_httpproxy.py
 tests/test_downloadermiddleware.py
 tests/test_downloadermiddleware_redirect.py
 tests/test_downloadermiddleware_retry.py
+tests/test_downloadermiddleware_stats.py
+tests/test_downloadermiddleware_useragent.py
 tests/test_engine.py
 tests/test_mail.py
 tests/test_pipeline_files.py
diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py
index 257eab609..13f35b92a 100644
--- a/tests/test_downloadermiddleware.py
+++ b/tests/test_downloadermiddleware.py
@@ -5,6 +5,7 @@ from scrapy.http import Request, Response
 from scrapy.spiders import Spider
 from scrapy.core.downloader.middleware import DownloaderMiddlewareManager
 from scrapy.utils.test import get_crawler
+from tests import mock
 
 
 class ManagerTestCase(TestCase):
@@ -90,3 +91,26 @@ class DefaultsTest(ManagerTestCase):
             'Location': 'http://example.com/login',
         })
         self.assertRaises(IOError, self._download, request=req, response=resp)
+
+
+class ResponseFromProcessRequestTest(ManagerTestCase):
+    """Tests middleware returning a response from process_request."""
+
+    def test_download_func_not_called(self):
+        resp = Response('http://example.com/index.html')
+
+        class ResponseMiddleware(object):
+            def process_request(self, request, spider):
+                return resp
+
+        self.mwman._add_middleware(ResponseMiddleware())
+
+        req = Request('http://example.com/index.html')
+        download_func = mock.MagicMock()
+        dfd = self.mwman.download(download_func, req, self.spider)
+        results = []
+        dfd.addBoth(results.append)
+        self._wait(dfd)
+
+        self.assertIs(results[0], resp)
+        self.assertFalse(download_func.called)
diff --git a/tests/test_downloadermiddleware_ajaxcrawlable.py b/tests/test_downloadermiddleware_ajaxcrawlable.py
index 493691ea4..67c57778d 100644
--- a/tests/test_downloadermiddleware_ajaxcrawlable.py
+++ b/tests/test_downloadermiddleware_ajaxcrawlable.py
@@ -14,7 +14,7 @@ class AjaxCrawlMiddlewareTest(unittest.TestCase):
         self.mw = AjaxCrawlMiddleware.from_crawler(crawler)
 
     def _ajaxcrawlable_body(self):
-        return b'<html><head><meta name="fragment" content="!"/></head><body></body></html>'
+        return '<html><head><meta name="fragment" content="!"/></head><body></body></html>'
 
     def _req_resp(self, url, req_kwargs=None, resp_kwargs=None):
         req = Request(url, **(req_kwargs or {}))
@@ -53,6 +53,6 @@ class AjaxCrawlMiddlewareTest(unittest.TestCase):
         assert resp3 is resp2
 
     def test_noncrawlable_body(self):
-        req, resp = self._req_resp('http://example.com/', {}, {'body': b'<html></html>'})
+        req, resp = self._req_resp('http://example.com/', {}, {'body': '<html></html>'})
         resp2 = self.mw.process_response(req, resp, self.spider)
         self.assertIs(resp, resp2)
diff --git a/tests/test_downloadermiddleware_defaultheaders.py b/tests/test_downloadermiddleware_defaultheaders.py
index 80efa83f9..75d8a1921 100644
--- a/tests/test_downloadermiddleware_defaultheaders.py
+++ b/tests/test_downloadermiddleware_defaultheaders.py
@@ -1,10 +1,10 @@
 from unittest import TestCase
+import six
 
 from scrapy.downloadermiddlewares.defaultheaders import DefaultHeadersMiddleware
 from scrapy.http import Request
 from scrapy.spiders import Spider
 from scrapy.utils.test import get_crawler
-from scrapy.utils.python import to_bytes
 
 
 class TestDefaultHeadersMiddleware(TestCase):
@@ -12,10 +12,8 @@ class TestDefaultHeadersMiddleware(TestCase):
     def get_defaults_spider_mw(self):
         crawler = get_crawler(Spider)
         spider = crawler._create_spider('foo')
-        defaults = {
-            to_bytes(k): [to_bytes(v)]
-            for k, v in crawler.settings.get('DEFAULT_REQUEST_HEADERS').items()
-        }
+        defaults = dict([(k, [v]) for k, v in \
+            six.iteritems(crawler.settings.get('DEFAULT_REQUEST_HEADERS'))])
         return defaults, spider, DefaultHeadersMiddleware.from_crawler(crawler)
 
     def test_process_request(self):
@@ -27,10 +25,9 @@ class TestDefaultHeadersMiddleware(TestCase):
     def test_update_headers(self):
         defaults, spider, mw = self.get_defaults_spider_mw()
         headers = {'Accept-Language': ['es'], 'Test-Header': ['test']}
-        bytes_headers = {b'Accept-Language': [b'es'], b'Test-Header': [b'test']}
         req = Request('http://www.scrapytest.org', headers=headers)
-        self.assertEquals(req.headers, bytes_headers)
+        self.assertEquals(req.headers, headers)
 
         mw.process_request(req, spider)
-        defaults.update(bytes_headers)
+        defaults.update(headers)
         self.assertEquals(req.headers, defaults)
diff --git a/tests/test_downloadermiddleware_httpauth.py b/tests/test_downloadermiddleware_httpauth.py
index 425a5cc79..c30fa97c6 100644
--- a/tests/test_downloadermiddleware_httpauth.py
+++ b/tests/test_downloadermiddleware_httpauth.py
@@ -4,12 +4,10 @@ from scrapy.http import Request
 from scrapy.downloadermiddlewares.httpauth import HttpAuthMiddleware
 from scrapy.spiders import Spider
 
-
 class TestSpider(Spider):
     http_user = 'foo'
     http_pass = 'bar'
 
-
 class HttpAuthMiddlewareTest(unittest.TestCase):
 
     def setUp(self):
@@ -23,10 +21,13 @@ class HttpAuthMiddlewareTest(unittest.TestCase):
     def test_auth(self):
         req = Request('http://scrapytest.org/')
         assert self.mw.process_request(req, self.spider) is None
-        self.assertEquals(req.headers['Authorization'], b'Basic Zm9vOmJhcg==')
+        self.assertEquals(req.headers['Authorization'], 'Basic Zm9vOmJhcg==')
 
     def test_auth_already_set(self):
-        req = Request('http://scrapytest.org/',
-                      headers=dict(Authorization='Digest 123'))
+        req = Request('http://scrapytest.org/', headers=dict(Authorization='Digest 123'))
         assert self.mw.process_request(req, self.spider) is None
-        self.assertEquals(req.headers['Authorization'], b'Digest 123')
+        self.assertEquals(req.headers['Authorization'], 'Digest 123')
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py
index b9c002f85..5f45dcb82 100644
--- a/tests/test_downloadermiddleware_robotstxt.py
+++ b/tests/test_downloadermiddleware_robotstxt.py
@@ -1,10 +1,11 @@
 from __future__ import absolute_import
 import re
 from twisted.internet import reactor, error
-from twisted.internet.defer import Deferred
+from twisted.internet.defer import Deferred, DeferredList, maybeDeferred
 from twisted.python import failure
 from twisted.trial import unittest
-from scrapy.downloadermiddlewares.robotstxt import RobotsTxtMiddleware
+from scrapy.downloadermiddlewares.robotstxt import (RobotsTxtMiddleware,
+                                                    logger as mw_module_logger)
 from scrapy.exceptions import IgnoreRequest, NotConfigured
 from scrapy.http import Request, Response, TextResponse
 from scrapy.settings import Settings
@@ -44,32 +45,26 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
 
     def test_robotstxt(self):
         middleware = RobotsTxtMiddleware(self._get_successful_crawler())
-        # There is a bit of neglect in robotstxt.py: robots.txt is fetched asynchronously,
-        # and it is actually fetched only *after* first process_request completes.
-        # So, first process_request will always succeed.
-        # We defer test() because otherwise robots.txt download mock will be called after assertRaises failure.
-        self.assertNotIgnored(Request('http://site.local'), middleware)
-        def test(r):
-            self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
-            self.assertIgnored(Request('http://site.local/admin/main'), middleware)
+        return DeferredList([
+            self.assertNotIgnored(Request('http://site.local/allowed'), middleware),
+            self.assertIgnored(Request('http://site.local/admin/main'), middleware),
             self.assertIgnored(Request('http://site.local/static/'), middleware)
-        deferred = Deferred()
-        deferred.addCallback(test)
-        reactor.callFromThread(deferred.callback, None)
-        return deferred
+        ], fireOnOneErrback=True)
+
+    def test_robotstxt_ready_parser(self):
+        middleware = RobotsTxtMiddleware(self._get_successful_crawler())
+        d = self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
+        d.addCallback(lambda _: self.assertNotIgnored(Request('http://site.local/allowed'), middleware))
+        return d
 
     def test_robotstxt_meta(self):
         middleware = RobotsTxtMiddleware(self._get_successful_crawler())
         meta = {'dont_obey_robotstxt': True}
-        self.assertNotIgnored(Request('http://site.local', meta=meta), middleware)
-        def test(r):
-            self.assertNotIgnored(Request('http://site.local/allowed', meta=meta), middleware)
-            self.assertNotIgnored(Request('http://site.local/admin/main', meta=meta), middleware)
+        return DeferredList([
+            self.assertNotIgnored(Request('http://site.local/allowed', meta=meta), middleware),
+            self.assertNotIgnored(Request('http://site.local/admin/main', meta=meta), middleware),
             self.assertNotIgnored(Request('http://site.local/static/', meta=meta), middleware)
-        deferred = Deferred()
-        deferred.addCallback(test)
-        reactor.callFromThread(deferred.callback, None)
-        return deferred
+        ], fireOnOneErrback=True)
 
     def _get_garbage_crawler(self):
         crawler = self.crawler
@@ -85,17 +80,12 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
     def test_robotstxt_garbage(self):
         # garbage response should be discarded, equal 'allow all'
         middleware = RobotsTxtMiddleware(self._get_garbage_crawler())
-        middleware._logerror = mock.MagicMock()
-        middleware.process_request(Request('http://site.local'), None)
-        self.assertNotIgnored(Request('http://site.local'), middleware)
-        def test(r):
-            self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
-            self.assertNotIgnored(Request('http://site.local/admin/main'), middleware)
+        deferred = DeferredList([
+            self.assertNotIgnored(Request('http://site.local'), middleware),
+            self.assertNotIgnored(Request('http://site.local/allowed'), middleware),
+            self.assertNotIgnored(Request('http://site.local/admin/main'), middleware),
             self.assertNotIgnored(Request('http://site.local/static/'), middleware)
-        deferred = Deferred()
-        deferred.addCallback(test)
-        deferred.addErrback(lambda _: self.assertIsNone(middleware._logerror.assert_any_call()))
-        reactor.callFromThread(deferred.callback, None)
+        ], fireOnOneErrback=True)
         return deferred
 
     def _get_emptybody_crawler(self):
@@ -112,15 +102,11 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
     def test_robotstxt_empty_response(self):
         # empty response should equal 'allow all'
         middleware = RobotsTxtMiddleware(self._get_emptybody_crawler())
-        self.assertNotIgnored(Request('http://site.local'), middleware)
-        def test(r):
-            self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
-            self.assertNotIgnored(Request('http://site.local/admin/main'), middleware)
+        return DeferredList([
+            self.assertNotIgnored(Request('http://site.local/allowed'), middleware),
+            self.assertNotIgnored(Request('http://site.local/admin/main'), middleware),
             self.assertNotIgnored(Request('http://site.local/static/'), middleware)
-        deferred = Deferred()
-        deferred.addCallback(test)
-        reactor.callFromThread(deferred.callback, None)
-        return deferred
+        ], fireOnOneErrback=True)
 
     def test_robotstxt_error(self):
         self.crawler.settings.set('ROBOTSTXT_OBEY', True)
@@ -132,17 +118,33 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
         self.crawler.engine.download.side_effect = return_failure
 
         middleware = RobotsTxtMiddleware(self.crawler)
-        middleware._logerror = mock.MagicMock()
-        middleware.process_request(Request('http://site.local'), None)
-        deferred = Deferred()
-        deferred.addErrback(lambda _: self.assertIsNone(middleware._logerror.assert_any_call()))
-        reactor.callFromThread(deferred.callback, None)
+        middleware._logerror = mock.MagicMock(side_effect=middleware._logerror)
+        deferred = middleware.process_request(Request('http://site.local'), None)
+        deferred.addCallback(lambda _: self.assertTrue(middleware._logerror.called))
         return deferred
 
+    def test_ignore_robotstxt_request(self):
+        self.crawler.settings.set('ROBOTSTXT_OBEY', True)
+        def ignore_request(request, spider):
+            deferred = Deferred()
+            reactor.callFromThread(deferred.errback, failure.Failure(IgnoreRequest()))
+            return deferred
+        self.crawler.engine.download.side_effect = ignore_request
+
+        middleware = RobotsTxtMiddleware(self.crawler)
+        mw_module_logger.error = mock.MagicMock()
+
+        d = self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
+        d.addCallback(lambda _: self.assertFalse(mw_module_logger.error.called))
+        return d
+
     def assertNotIgnored(self, request, middleware):
         spider = None  # not actually used
-        self.assertIsNone(middleware.process_request(request, spider))
+        dfd = maybeDeferred(middleware.process_request, request, spider)
+        dfd.addCallback(self.assertIsNone)
+        return dfd
 
     def assertIgnored(self, request, middleware):
         spider = None  # not actually used
-        self.assertRaises(IgnoreRequest, middleware.process_request, request, spider)
+        return self.assertFailure(maybeDeferred(middleware.process_request, request, spider),
+                                  IgnoreRequest)
diff --git a/tests/test_downloadermiddleware_stats.py b/tests/test_downloadermiddleware_stats.py
index 1f2616e35..fb46ccff6 100644
--- a/tests/test_downloadermiddleware_stats.py
+++ b/tests/test_downloadermiddleware_stats.py
@@ -6,10 +6,6 @@ from scrapy.spiders import Spider
 from scrapy.utils.test import get_crawler
 
 
-class MyException(Exception):
-    pass
-
-
 class TestDownloaderStats(TestCase):
 
     def setUp(self):
@@ -22,28 +18,21 @@ class TestDownloaderStats(TestCase):
         self.req = Request('http://scrapytest.org')
         self.res = Response('scrapytest.org', status=400)
 
-    def assertStatsEqual(self, key, value):
-        self.assertEqual(
-            self.crawler.stats.get_value(key, spider=self.spider),
-            value,
-            str(self.crawler.stats.get_stats(self.spider))
-        )
-
     def test_process_request(self):
         self.mw.process_request(self.req, self.spider)
-        self.assertStatsEqual('downloader/request_count', 1)
+        self.assertEqual(self.crawler.stats.get_value('downloader/request_count', \
+            spider=self.spider), 1)
 
     def test_process_response(self):
         self.mw.process_response(self.req, self.res, self.spider)
-        self.assertStatsEqual('downloader/response_count', 1)
+        self.assertEqual(self.crawler.stats.get_value('downloader/response_count', \
+            spider=self.spider), 1)
 
     def test_process_exception(self):
-        self.mw.process_exception(self.req, MyException(), self.spider)
-        self.assertStatsEqual('downloader/exception_count', 1)
-        self.assertStatsEqual(
-            'downloader/exception_type_count/tests.test_downloadermiddleware_stats.MyException',
-            1
-        )
+        self.mw.process_exception(self.req, Exception(), self.spider)
+        self.assertEqual(self.crawler.stats.get_value('downloader/exception_count', \
+            spider=self.spider), 1)
 
     def tearDown(self):
         self.crawler.stats.close_spider(self.spider, '')
+
diff --git a/tests/test_downloadermiddleware_useragent.py b/tests/test_downloadermiddleware_useragent.py
index 1e41fdace..741c8de76 100644
--- a/tests/test_downloadermiddleware_useragent.py
+++ b/tests/test_downloadermiddleware_useragent.py
@@ -17,7 +17,7 @@ class UserAgentMiddlewareTest(TestCase):
         spider, mw = self.get_spider_and_mw('default_useragent')
         req = Request('http://scrapytest.org/')
         assert mw.process_request(req, spider) is None
-        self.assertEquals(req.headers['User-Agent'], b'default_useragent')
+        self.assertEquals(req.headers['User-Agent'], 'default_useragent')
 
     def test_remove_agent(self):
         # settings UESR_AGENT to None should remove the user agent
@@ -34,16 +34,15 @@ class UserAgentMiddlewareTest(TestCase):
         mw.spider_opened(spider)
         req = Request('http://scrapytest.org/')
         assert mw.process_request(req, spider) is None
-        self.assertEquals(req.headers['User-Agent'], b'spider_useragent')
+        self.assertEquals(req.headers['User-Agent'], 'spider_useragent')
 
     def test_header_agent(self):
         spider, mw = self.get_spider_and_mw('default_useragent')
         spider.user_agent = 'spider_useragent'
         mw.spider_opened(spider)
-        req = Request('http://scrapytest.org/',
-                      headers={'User-Agent': 'header_useragent'})
+        req = Request('http://scrapytest.org/', headers={'User-Agent': 'header_useragent'})
         assert mw.process_request(req, spider) is None
-        self.assertEquals(req.headers['User-Agent'], b'header_useragent')
+        self.assertEquals(req.headers['User-Agent'], 'header_useragent')
 
     def test_no_agent(self):
         spider, mw = self.get_spider_and_mw(None)
diff --git a/tests/test_http_request.py b/tests/test_http_request.py
index ff0941961..b87045325 100644
--- a/tests/test_http_request.py
+++ b/tests/test_http_request.py
@@ -767,40 +767,6 @@ class FormRequestTest(RequestTest):
         self.assertEqual(fs[b'test2'], [b'val2'])
         self.assertEqual(fs[b'button1'], [b'submit1'])
 
-    def test_from_response_submit_novalue(self):
-        response = _buildresponse(
-            """<form action="post.php" method="POST">
-            <input type="hidden" name="test1" value="val1">
-            <input type="hidden" name="test2" value="val2">
-            <input type="submit" name="button1">Submit</button>
-            </form>""",
-            url="http://www.example.com/this/list.html")
-        req = self.request_class.from_response(response)
-        self.assertEqual(req.method, 'POST')
-        self.assertEqual(req.headers['Content-type'], b'application/x-www-form-urlencoded')
-        self.assertEqual(req.url, "http://www.example.com/this/post.php")
-        fs = _qs(req)
-        self.assertEqual(fs[b'test1'], [b'val1'])
-        self.assertEqual(fs[b'test2'], [b'val2'])
-        self.assertEqual(fs[b'button1'], [b''])
-
-    def test_from_response_button_novalue(self):
-        response = _buildresponse(
-            """<form action="post.php" method="POST">
-            <input type="hidden" name="test1" value="val1">
-            <input type="hidden" name="test2" value="val2">
-            <button type="submit" name="button1">Submit</button>
-            </form>""",
-            url="http://www.example.com/this/list.html")
-        req = self.request_class.from_response(response)
-        self.assertEqual(req.method, 'POST')
-        self.assertEqual(req.headers['Content-type'], b'application/x-www-form-urlencoded')
-        self.assertEqual(req.url, "http://www.example.com/this/post.php")
-        fs = _qs(req)
-        self.assertEqual(fs[b'test1'], [b'val1'])
-        self.assertEqual(fs[b'test2'], [b'val2'])
-        self.assertEqual(fs[b'button1'], [b''])
-
 def _buildresponse(body, **kwargs):
     kwargs.setdefault('body', body)
     kwargs.setdefault('url', 'http://example.com')