Repository: scrapinghub/scrapy-autounit

Issue:
Warn if CONCURRENT_REQUESTS > 1
Concurrency could affect the results right, if there's a shared object or shared spider attribute or something.  I think this can be detected at startup, so it might be good to alert the user.

Discussion:

Patch:
diff --git a/scrapy_autounit/middleware.py b/scrapy_autounit/middleware.py
index 0568541..dd02de8 100644
--- a/scrapy_autounit/middleware.py
+++ b/scrapy_autounit/middleware.py
@@ -1,9 +1,11 @@
 import six
 import random
 from pathlib import Path
+import logging
 
 from scrapy.http import Request
 from scrapy.exceptions import NotConfigured
+from scrapy.commands.genspider import sanitize_module_name
 
 from .utils import (
     add_sample,
@@ -16,6 +18,8 @@ from .utils import (
     create_dir,
 )
 
+logger = logging.getLogger(__name__)
+
 
 def _copy_settings(settings):
     out = {}
@@ -26,8 +30,21 @@ def _copy_settings(settings):
 
 class AutounitMiddleware:
     def __init__(self, settings):
+        if not any(
+            self.__class__.__name__ in s
+            for s in settings.getwithbase('SPIDER_MIDDLEWARES').keys()
+        ):
+            raise ValueError(
+                '%s must be in SPIDER_MIDDLEWARES' % (
+                    self.__class__.__name__,))
         if not settings.getbool('AUTOUNIT_ENABLED'):
             raise NotConfigured('scrapy-autounit is not enabled')
+        if settings.getint('CONCURRENT_REQUESTS') > 1:
+            logger.warn(
+                'Recording with concurrency > 1! '
+                'Data races in shared object modification may create broken '
+                'tests.'
+            )
 
         self.max_fixtures = settings.getint(
             'AUTOUNIT_MAX_FIXTURES_PER_CALLBACK',
@@ -91,7 +108,7 @@ class AutounitMiddleware:
 
         test_dir, test_name = get_or_create_test_dir(
             self.base_path,
-            spider.name,
+            sanitize_module_name(spider.name),
             callback_name,
             settings.get('AUTOUNIT_EXTRA_PATH'),
         )


Test patch:
diff --git a/tests/test_record.py b/tests/test_record.py
index a4865ad..33ed48e 100644
--- a/tests/test_record.py
+++ b/tests/test_record.py
@@ -10,7 +10,7 @@ import scrapy
 
 
 class MySpider(scrapy.Spider):
-    name = 'myspider'
+    name = '{name}'
 
     custom_settings = dict(
         SPIDER_MIDDLEWARES={{
@@ -66,6 +66,7 @@ class CaseSpider(object):
             dest.write('SPIDER_MODULES = ["myproject"]\n')
         self._start_requests = None
         self._parse = None
+        self._spider_name = 'myspider'
 
     def __enter__(self):
         return self
@@ -73,17 +74,19 @@ class CaseSpider(object):
     def __exit__(self, exc_type, exc_value, traceback):
         shutil.rmtree(self.dir)
 
+    def name(self, string):
+        self._spider_name = string
+
     def start_requests(self, string):
         self._start_requests = string
-        self._write_spider()
 
     def parse(self, string):
         self._parse = string
-        self._write_spider()
 
     def _write_spider(self):
         with open(os.path.join(self.proj_dir, 'myspider.py'), 'w') as dest:
             dest.write(SPIDER_TEMPLATE.format(
+                name=self._spider_name,
                 start_requests=self._start_requests,
                 parse=self._parse
             ))
@@ -91,11 +94,12 @@ class CaseSpider(object):
     def record(self, args=None, settings=None):
         if self._start_requests is None or self._parse is None:
             raise AssertionError()
+        self._write_spider()
         env = os.environ.copy()
         env['PYTHONPATH'] = self.dir  # doesn't work if == cwd
         env['SCRAPY_SETTINGS_MODULE'] = 'myproject.settings'
         command_args = [
-            'scrapy', 'crawl', 'myspider',
+            'scrapy', 'crawl', self._spider_name,
             '-s', 'AUTOUNIT_ENABLED=1',
         ]
         for k, v in (args or {}).items():
@@ -160,3 +164,18 @@ class TestRecording(unittest.TestCase):
             ''')
             spider.record(settings=dict(AUTOUNIT_EXTRA_PATH='abc'))
             spider.test()
+
+    def test_empty(self):
+        with CaseSpider() as spider:
+            spider.start_requests("yield scrapy.Request('data:text/plain,')")
+            spider.parse('pass')
+            spider.record()
+            spider.test()
+
+    def test_dotted_name(self):
+        with CaseSpider() as spider:
+            spider.name('my.spider')
+            spider.start_requests("yield scrapy.Request('data:text/plain,')")
+            spider.parse('pass')
+            spider.record()
+            spider.test()