Merge pull request #2612 from redapple/dupe-spider-name-tests

[WIP] Add warning on duplicate spider name

scrapy/spiderloader.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import
+from collections import defaultdict
 import traceback
 import warnings
 
@@ -19,10 +20,24 @@ class SpiderLoader(object):
     def __init__(self, settings):
         self.spider_modules = settings.getlist('SPIDER_MODULES')
         self._spiders = {}
+        self._found = defaultdict(list)
         self._load_all_spiders()
 
+    def _check_name_duplicates(self):
+        dupes = ["\n".join("  {cls} named {name!r} (in {module})".format(
+                                module=mod, cls=cls, name=name)
+                           for (mod, cls) in locations)
+                 for name, locations in self._found.items()
+                 if len(locations)>1]
+        if dupes:
+            msg = ("There are several spiders with the same name:\n\n"
+                   "{}\n\n  This can cause unexpected behavior.".format(
+                        "\n\n".join(dupes)))
+            warnings.warn(msg, UserWarning)
+
     def _load_spiders(self, module):
         for spcls in iter_spider_classes(module):
+            self._found[spcls.name].append((module.__name__, spcls.__name__))
             self._spiders[spcls.name] = spcls
 
     def _load_all_spiders(self):
@@ -35,6 +50,7 @@ class SpiderLoader(object):
                        "Check SPIDER_MODULES setting".format(
                             modname=name, tb=traceback.format_exc()))
                 warnings.warn(msg, RuntimeWarning)
+        self._check_name_duplicates()
 
     @classmethod
     def from_settings(cls, settings):

tests/test_spiderloader/__init__.py

@@ -101,3 +101,55 @@ class SpiderLoaderTest(unittest.TestCase):
 
             spiders = spider_loader.list()
             self.assertEqual(spiders, [])
+
+
+class DuplicateSpiderNameLoaderTest(unittest.TestCase):
+
+    def setUp(self):
+        orig_spiders_dir = os.path.join(module_dir, 'test_spiders')
+        self.tmpdir = self.mktemp()
+        os.mkdir(self.tmpdir)
+        self.spiders_dir = os.path.join(self.tmpdir, 'test_spiders_xxx')
+        shutil.copytree(orig_spiders_dir, self.spiders_dir)
+        sys.path.append(self.tmpdir)
+        self.settings = Settings({'SPIDER_MODULES': ['test_spiders_xxx']})
+
+    def tearDown(self):
+        del sys.modules['test_spiders_xxx']
+        sys.path.remove(self.tmpdir)
+
+    def test_dupename_warning(self):
+        # copy 1 spider module so as to have duplicate spider name
+        shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx/spider3.py'),
+                        os.path.join(self.tmpdir, 'test_spiders_xxx/spider3dupe.py'))
+
+        with warnings.catch_warnings(record=True) as w:
+            spider_loader = SpiderLoader.from_settings(self.settings)
+
+            self.assertEqual(len(w), 1)
+            msg = str(w[0].message)
+            self.assertIn("several spiders with the same name", msg)
+            self.assertIn("'spider3'", msg)
+
+            spiders = set(spider_loader.list())
+            self.assertEqual(spiders, set(['spider1', 'spider2', 'spider3', 'spider4']))
+
+    def test_multiple_dupename_warning(self):
+        # copy 2 spider modules so as to have duplicate spider name
+        # This should issue 2 warning, 1 for each duplicate spider name
+        shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx/spider1.py'),
+                        os.path.join(self.tmpdir, 'test_spiders_xxx/spider1dupe.py'))
+        shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx/spider2.py'),
+                        os.path.join(self.tmpdir, 'test_spiders_xxx/spider2dupe.py'))
+
+        with warnings.catch_warnings(record=True) as w:
+            spider_loader = SpiderLoader.from_settings(self.settings)
+
+            self.assertEqual(len(w), 1)
+            msg = str(w[0].message)
+            self.assertIn("several spiders with the same name", msg)
+            self.assertIn("'spider1'", msg)
+            self.assertIn("'spider2'", msg)
+
+            spiders = set(spider_loader.list())
+            self.assertEqual(spiders, set(['spider1', 'spider2', 'spider3', 'spider4']))