Repository: scrapinghub/scrapy-autounit

Issue:
Spider attributes
So this goes along with the mention of untracked dependencies (#14), but I have a spider that has a `page_number` attribute that's incremented after every `yield self.next_page()` call.  It's initialized to 0, so when recording a callback might request page 10, but during playback the same code now requests page 0.

There's a solution to this, which is to pass `page_number` to the callback via `meta`, and since there's no branching (the increment always happens in one callback which is requested by the callback that did the previous increment) this works and isn't too difficult.

But I wonder if autounit could track spider attributes?

Discussion:
mabelvj: Hi @fcanobrash! Thanks for your comments! I will update the PR. I know not all the code is related to the issue, but for understanding the code I had to develop some code and I thought it would be a good improvement.

Patch:
diff --git a/scrapy_autounit/middleware.py b/scrapy_autounit/middleware.py
index fdf8e5a..160bf3a 100644
--- a/scrapy_autounit/middleware.py
+++ b/scrapy_autounit/middleware.py
@@ -103,13 +103,18 @@ class AutounitMiddleware:
 
         request = input_data['request']
         callback_name = request['callback']
+        spider_attr_out = {
+            k: v for k, v in spider.__dict__.items()
+            if k not in ('crawler', 'settings', 'start_urls')
+        }
 
         data = {
             'spider_name': spider.name,
             'request': request,
             'response': input_data['response'],
+            'spider_args_out': spider_attr_out,
             'result': processed_result,
-            'spider_args': input_data['spider_args'],
+            'spider_args_in': input_data['spider_args'],
             'settings': _copy_settings(settings),
             'middlewares': input_data['middlewares'],
             'python_version': 2 if six.PY2 else 3,
diff --git a/scrapy_autounit/utils.py b/scrapy_autounit/utils.py
index 1529a32..fc28858 100644
--- a/scrapy_autounit/utils.py
+++ b/scrapy_autounit/utils.py
@@ -1,33 +1,26 @@
 import os
-import six
+import pickle
 import sys
 import zlib
-import pickle
-from pathlib import Path
-from itertools import islice
 from importlib import import_module
+from itertools import islice
+from pathlib import Path
 
+import six
 from scrapy import signals
-from scrapy.item import Item
 from scrapy.crawler import Crawler
 from scrapy.exceptions import NotConfigured
 from scrapy.http import Request, Response
+from scrapy.item import Item
+from scrapy.utils.conf import (build_component_list, closest_scrapy_cfg,
+                               init_env)
+from scrapy.utils.misc import arg_to_iter, load_object, walk_modules
+from scrapy.utils.project import get_project_settings
 from scrapy.utils.python import to_bytes
-from scrapy.utils.reqser import request_to_dict, request_from_dict
+from scrapy.utils.reqser import request_from_dict, request_to_dict
 from scrapy.utils.spider import iter_spider_classes
-from scrapy.utils.project import get_project_settings
-from scrapy.utils.misc import (
-    walk_modules,
-    load_object,
-    arg_to_iter,
-)
-from scrapy.utils.conf import (
-    init_env,
-    closest_scrapy_cfg,
-    build_component_list,
-)
-import datadiff.tools
 
+import datadiff.tools
 
 NO_ITEM_MARKER = object()
 FIXTURE_VERSION = 1
@@ -217,7 +210,7 @@ def _clean(data, settings, name):
         data.pop(field, None)
 
 
-def write_test(path, test_name, fixture_name, url):
+def write_test(path, test_name, fixture_name, encoding, url):
     command = 'scrapy {}'.format(' '.join(sys.argv))
     test_path = path / ('test_%s.py' % (fixture_name))
 
@@ -252,6 +245,7 @@ if __name__ == '__main__':
         fixture_name=fixture_name,
         command=command,
         url=url,
+        encoding=encoding,
     )
 
     with open(str(test_path), 'w') as f:
@@ -285,6 +279,11 @@ def binary_check(fx_obj, cb_obj, encoding):
     return fx_obj
 
 
+def set_spider_attrs(spider, _args):
+    for k, v in _args.items():
+        setattr(spider, k, v)
+
+
 def generate_test(fixture_path, encoding='utf-8'):
     with open(str(fixture_path), 'rb') as f:
         raw_data = f.read()
@@ -306,14 +305,19 @@ def generate_test(fixture_path, encoding='utf-8'):
     spider_cls.update_settings(settings)
     for k, v in data.get('settings', {}).items():
         settings.set(k, v, 50)
+
     crawler = Crawler(spider_cls, settings)
-    spider = spider_cls.from_crawler(crawler, **data.get('spider_args'))
+    spider_args_in = data.get('spider_args', data.get('spider_args_in', {}))
+    spider = spider_cls.from_crawler(crawler, **spider_args_in)
     crawler.spider = spider
 
     def test(self):
         fx_result = data['result']
         fx_version = data.get('python_version')
 
+        spider_args_in = data.get(
+            'spider_args', data.get('spider_args_in', {}))
+        set_spider_attrs(spider, spider_args_in)
         request = request_from_dict(data['request'], spider)
         response_cls = auto_import(data['response'].pop(
             'cls', 'scrapy.http.HtmlResponse'))
@@ -333,6 +337,11 @@ def generate_test(fixture_path, encoding='utf-8'):
             signal=signals.spider_opened,
             spider=spider
         )
+        result_attr_in = {
+            k: v for k, v in spider.__dict__.items()
+            if k not in ('crawler', 'settings', 'start_urls')
+        }
+        self.assertEqual(spider_args_in, result_attr_in, 'Not equal!')
 
         for mw in middlewares:
             if hasattr(mw, 'process_spider_input'):
@@ -376,4 +385,12 @@ def generate_test(fixture_path, encoding='utf-8'):
                         "output:{}".format(index, e)),
                     None)
 
+        # Spider attributes get updated after the yield
+        result_attr_out = {
+            k: v for k, v in spider.__dict__.items()
+            if k not in ('crawler', 'settings', 'start_urls')
+        }
+
+        self.assertEqual(data['spider_args_out'], result_attr_out, 'Not equal!'
+                         )
     return test


Test patch:
diff --git a/tests/test_record.py b/tests/test_record.py
index 4e60bc0..15264a2 100644
--- a/tests/test_record.py
+++ b/tests/test_record.py
@@ -21,6 +21,10 @@ class MySpider(scrapy.Spider):
         {custom_settings}
     )
 
+    def __init__(self, *args, **kwargs):
+        {init}
+        super(MySpider, self).__init__(*args, **kwargs)
+
     def start_requests(self):
         {start_requests}
 
@@ -64,6 +68,28 @@ def check_process(message, result):
     process_error(message, result)
 
 
+def indent_message(text):
+    _text = re.sub(r'(\n)(\n*)', r'\n\t', text)
+    return re.sub(r'^([\n\t]*)(.*)', r'\t\2', _text)
+
+
+def print_test_output(result):
+    # Print the output of the tests
+    print('\n')
+    print(indent_message(''.join(result['stdout'].decode())))
+    print(indent_message(''.join(result['stderr'].decode())))
+
+
+def itertree(startpath):
+    for root, dirs, files in os.walk(startpath):
+        level = root.replace(startpath, '').count(os.sep)
+        indent = ' ' * 4 * (level)
+        yield('{}{}/'.format(indent, os.path.basename(root)))
+        subindent = ' ' * 4 * (level + 1)
+        for f in files:
+            yield('{}{}'.format(subindent, f))
+
+
 class CaseSpider(object):
     def __init__(self):
         self.dir = tempfile.mkdtemp()
@@ -79,6 +105,7 @@ class CaseSpider(object):
         self._imports = ''
         self._custom_settings = ''
         self._second_callback = None
+        self.init = None
 
     def __enter__(self):
         return self
@@ -95,6 +122,9 @@ class CaseSpider(object):
     def name(self, string):
         self._spider_name = string
 
+    def set_init(self, string):
+        self.init = string
+
     def start_requests(self, string):
         self._start_requests = string
 
@@ -108,14 +138,15 @@ class CaseSpider(object):
         with open(os.path.join(self.proj_dir, 'myspider.py'), 'w') as dest:
             dest.write(SPIDER_TEMPLATE.format(
                 name=self._spider_name,
+                init=self.init,
                 start_requests=self._start_requests,
                 parse=self._parse,
                 imports=self._imports,
                 custom_settings=self._custom_settings,
-                second_callback=self._second_callback,
+                second_callback=self._second_callback
             ))
 
-    def record(self, args=None, settings=None):
+    def record(self, args=None, settings=None, record_verbosity=False):
         if self._start_requests is None or self._parse is None:
             raise AssertionError()
         self._write_spider()
@@ -135,18 +166,20 @@ class CaseSpider(object):
         result = run(
             command_args,
             env=env,
-            cwd='/',
+            cwd=self.dir,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE
         )
         check_process('Running spider failed!', result)
+        if record_verbosity:
+            print_test_output(result)
         if not any(
             any(f.endswith('.py') and f != '__init__.py' for f in files)
             for _, _, files in os.walk(os.path.join(self.dir, 'autounit'))
         ):
             process_error('No autounit tests recorded!', result)
 
-    def test(self):
+    def test(self, test_verbosity=True):
         if self._start_requests is None or self._parse is None:
             raise AssertionError()
         env = os.environ.copy()
@@ -155,44 +188,147 @@ class CaseSpider(object):
             [
                 'python', '-m', 'unittest', 'discover', '-v'
             ],
-            cwd=self.dir,
             env=env,
-            stderr=subprocess.PIPE,
+            cwd=self.dir,
             stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE
         )
         check_process('Unit tests failed!', result)
         err = result['stderr'].decode('utf-8')
         tests_ran = re.search('Ran ([0-9]+) test', err).group(1)
-        if tests_ran == '0':
-            def itertree():
-                for root, dirs, files in os.walk(self.dir):
-                    for f in files:
-                        yield os.path.join(root, f)
-            raise AssertionError(
-                'No tests run!\nProject dir:\n{}'.format(
-                    '\n'.join(itertree())
-                ))
+
+        is_ok = re.findall('OK$', err)
+        if test_verbosity:
+            print_test_output(result)
+        if not is_ok or not tests_ran:
+            if not tests_ran:
+                raise AssertionError(
+                    'No tests run!\nProject dir:\n{}'.format(
+                        '\n'.join(itertree(self.dir))
+                    ))
+            elif not test_verbosity:
+                print_test_output(result)
 
 
 class TestRecording(unittest.TestCase):
     def test_normal(self):
         with CaseSpider() as spider:
             spider.start_requests("yield scrapy.Request('data:text/plain,')")
-            spider.parse('''
+            spider.parse("""
                 yield {'a': 4}
-            ''')
+            """)
             spider.record()
             spider.test()
 
     def test_path_extra(self):
         with CaseSpider() as spider:
             spider.start_requests("yield scrapy.Request('data:text/plain,')")
-            spider.parse('''
+            spider.parse("""
                 yield {'a': 4}
-            ''')
+            """)
             spider.record(settings=dict(AUTOUNIT_EXTRA_PATH='abc'))
             spider.test()
 
+    def test_spider_attributes(self):
+        with CaseSpider() as spider:
+            spider.start_requests("""
+                self._base_url = 'www.nothing.com'
+                yield scrapy.Request('data:text/plain,')
+            """)
+            spider.parse("""
+                self.param = 1
+                yield {'a': 4}
+            """)
+            spider.record()
+            spider.test()
+
+        with CaseSpider() as spider:
+            spider.start_requests("""
+                self._base_url = 'www.nothing.com'
+                yield scrapy.Request('data:text/plain,')
+            """)
+            spider.parse("""
+                self.param = 1
+                yield {'a': 4}
+            """)
+            spider.record(settings=dict(
+                AUTOUNIT_EXCLUDED_FIELDS='_base_url',
+                AUTOUNIT_INCLUDED_SETTINGS='AUTOUNIT_EXCLUDED_FIELDS'))
+            spider.test()
+
+    def test_spider_attributes_recursive(self):
+        # Recursive calls including private variables
+        with CaseSpider() as spider:
+            spider.start_requests("""
+                self.__page = 0
+                self.param = 0
+                self._base_url = 'www.nothing.com'
+                yield scrapy.Request('data:text/plain,', callback=self.parse)
+            """)
+            spider.parse("""
+                self.param += 1
+                reqs = self.second_callback(response)
+                for r in reqs:
+                    yield r
+            """)
+            spider.second_callback("""
+                self.__page += 1
+                if self.__page > 3:
+                    self.end = True
+                    yield {'a': 4}
+                    return
+                for i in range(3):
+                    yield {'b': '%s_%s;'%(self.__page, i)}
+                yield scrapy.Request('data:,%s;'%(self.__page),
+                                      callback=self.parse)
+                                         """)
+            spider.record()
+            spider.test()
+
+        # Recursive calls including private variables using getattr
+        with CaseSpider() as spider:
+            spider.start_requests("""
+                self.param = 0
+                self._base_url = 'www.nothing.com'
+                yield scrapy.Request('data:text/plain,', callback=self.parse)
+            """)
+            spider.parse("""
+                self.param += 1
+                reqs = self.second_callback(response)
+                for r in reqs:
+                    yield r
+            """)
+            spider.second_callback("""
+                self.__page = getattr(self, '_MySpider__page', 0) + 1
+                if self.__page > 3:
+                    self.end = True
+                    yield {'a': 4}
+                    return
+                for i in range(3):
+                    yield {'b': '%s_%s;'%(self.__page, i)}
+                yield scrapy.Request('data:,%s;'%(self.__page),
+                                      callback=self.parse)
+                                         """)
+            spider.record()
+            spider.test()
+
+        # Recursive calls including private variables using getattr
+        with CaseSpider() as spider:
+            spider.set_init("""self.page_number = 0""")
+            spider.start_requests("""
+                yield scrapy.Request('data:text/plain,')
+            """)
+            spider.parse("""
+                self.page_number += 1
+                yield {
+                    'page_number': self.page_number
+                }
+                if self.page_number < 3:
+                    yield scrapy.Request('data:text/plain,', dont_filter=True)
+            """)
+            spider.record()
+            spider.test()
+
     def test_empty(self):
         with CaseSpider() as spider:
             spider.start_requests("yield scrapy.Request('data:text/plain,')")