From 99b76eaa2cd347700de5a989656ec583b82ec460 Mon Sep 17 00:00:00 2001 From: Alex Cepoi Date: Tue, 21 Aug 2012 02:47:35 +0200 Subject: [PATCH 1/6] SEP-017 contracts: first draft --- .gitignore | 6 +++ scrapy/commands/check.py | 50 ++++++++++++++++++++++++ scrapy/contracts/__init__.py | 2 + scrapy/contracts/base.py | 74 ++++++++++++++++++++++++++++++++++++ scrapy/contracts/default.py | 23 +++++++++++ 5 files changed, 155 insertions(+) create mode 100644 scrapy/commands/check.py create mode 100644 scrapy/contracts/__init__.py create mode 100644 scrapy/contracts/base.py create mode 100644 scrapy/contracts/default.py diff --git a/.gitignore b/.gitignore index f7f30b06f..39eeda695 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,12 @@ *.pyc +*swp +*~ + _trial_temp dropin.cache docs/build *egg-info .tox + +build/ +dist/ diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py new file mode 100644 index 000000000..e68add5eb --- /dev/null +++ b/scrapy/commands/check.py @@ -0,0 +1,50 @@ +from functools import wraps + +from scrapy.command import ScrapyCommand +from scrapy.http import Request + +from scrapy.contracts import Contract + +class Command(ScrapyCommand): + requires_project = True + + def syntax(self): + return "[options] " + + def short_desc(self): + return "Check contracts for given spider" + + def run(self, args, opts): + self.crawler.engine.has_capacity = lambda: True + + for spider in args or self.crawler.spiders.list(): + spider = self.crawler.spiders.create(spider) + requests = self.get_requests(spider) + self.crawler.crawl(spider, requests) + + self.crawler.start() + + def get_requests(self, spider): + requests = [] + + for key, value in vars(type(spider)).iteritems(): + if callable(value) and value.__doc__: + bound_method = value.__get__(spider, type(spider)) + request = Request(url='http://scrapy.org', callback=bound_method) + + # register contract hooks to the request + contracts = Contract.from_method(value) + for contract in contracts: + request = contract.prepare_request(request) + + # discard anything the request might return + cb = request.callback + @wraps(cb) + def wrapper(response): + cb(response) + + request.callback = wrapper + + requests.append(request) + + return requests diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py new file mode 100644 index 000000000..fb66291c7 --- /dev/null +++ b/scrapy/contracts/__init__.py @@ -0,0 +1,2 @@ +from .base import Contract, ContractType +from .default import * diff --git a/scrapy/contracts/base.py b/scrapy/contracts/base.py new file mode 100644 index 000000000..f5756be31 --- /dev/null +++ b/scrapy/contracts/base.py @@ -0,0 +1,74 @@ +import re +from functools import wraps + +from scrapy.utils.spider import iterate_spider_output + +class ContractType(type): + """ Metaclass for contracts + - automatically registers contracts in the root `Contract` class + """ + + def __new__(meta, name, bases, dct): + # only allow single inheritence + assert len(bases) == 1, 'Multiple inheritance is not allowed' + base = bases[0] + + # ascend in inheritence chain + while type(base) not in [type, meta]: + base = type(base) + + # register this as a valid contract + cls = type.__new__(meta, name, bases, dct) + if type(base) != type: + base.registered[cls.name] = cls + return cls + + +class Contract(object): + """ Abstract class for contracts + - keeps a reference of all derived classes in `registered` + """ + + __metaclass__ = ContractType + registered = {} + + def __init__(self, method, *args): + self.method = method + self.args = args + + @classmethod + def from_method(cls, method): + contracts = [] + for line in method.__doc__.split('\n'): + line = line.strip() + + if line.startswith('@'): + name, args = re.match(r'@(\w+)\s*(.*)', line).groups() + args = re.split(r'[\,\s+]', args) + args = filter(lambda x:x, args) + + contracts.append(cls.registered[name](method, *args)) + + return contracts + + def prepare_request(self, request): + cb = request.callback + @wraps(cb) + def wrapper(response): + self.pre_process(response) + output = list(iterate_spider_output(cb(response))) + self.post_process(output) + return output + + request.callback = wrapper + request = self.modify_request(request) + return request + + def modify_request(self, request): + return request + + def pre_process(self, response): + pass + + def post_process(self, output): + pass diff --git a/scrapy/contracts/default.py b/scrapy/contracts/default.py new file mode 100644 index 000000000..77f2e8c75 --- /dev/null +++ b/scrapy/contracts/default.py @@ -0,0 +1,23 @@ +from scrapy.item import BaseItem + +from .base import Contract + + +# contracts +class UrlContract(Contract): + name = 'url' + + def modify_request(self, request): + return request.replace(url=self.args[0]) + +class ReturnsRequestContract(Contract): + name = 'returns_request' + +class ScrapesContract(Contract): + name = 'scrapes' + + def post_process(self, output): + for x in output: + if isinstance(x, BaseItem): + for arg in self.args: + assert arg in x, '%r field is missing' % arg From 901987154eebcbaa1ce3e37b7c31892aae353d57 Mon Sep 17 00:00:00 2001 From: Alex Cepoi Date: Fri, 24 Aug 2012 16:42:13 +0200 Subject: [PATCH 2/6] SEP-017 contracts * load contracts from settings * refactored contracts manager * fixed callback bug, which caused responses to be evaluated with a wrong callback sometimes * "returns" contract --- scrapy/commands/check.py | 47 ++++++++++------- scrapy/contracts/__init__.py | 78 ++++++++++++++++++++++++++++- scrapy/contracts/base.py | 74 --------------------------- scrapy/contracts/default.py | 66 +++++++++++++++++++++--- scrapy/exceptions.py | 3 ++ scrapy/settings/default_settings.py | 7 +++ scrapy/utils/misc.py | 15 ++++++ 7 files changed, 191 insertions(+), 99 deletions(-) delete mode 100644 scrapy/contracts/base.py diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py index e68add5eb..d548e472c 100644 --- a/scrapy/commands/check.py +++ b/scrapy/commands/check.py @@ -1,9 +1,21 @@ from functools import wraps +from scrapy.conf import settings from scrapy.command import ScrapyCommand from scrapy.http import Request +from scrapy.contracts import ContractsManager +from scrapy.utils import display +from scrapy.utils.misc import load_object +from scrapy.utils.spider import iterate_spider_output -from scrapy.contracts import Contract +def _generate(cb): + """ create a callback which does not return anything """ + @wraps(cb) + def wrapper(response): + output = cb(response) + output = list(iterate_spider_output(output)) + # display.pprint(output) + return wrapper class Command(ScrapyCommand): requires_project = True @@ -15,6 +27,17 @@ class Command(ScrapyCommand): return "Check contracts for given spider" def run(self, args, opts): + self.conman = ContractsManager() + + # load contracts + contracts = settings['SPIDER_CONTRACTS_BASE'] + \ + settings['SPIDER_CONTRACTS'] + + for contract in contracts: + concls = load_object(contract) + self.conman.register(concls) + + # schedule requests self.crawler.engine.has_capacity = lambda: True for spider in args or self.crawler.spiders.list(): @@ -22,29 +45,19 @@ class Command(ScrapyCommand): requests = self.get_requests(spider) self.crawler.crawl(spider, requests) + # start checks self.crawler.start() def get_requests(self, spider): requests = [] - for key, value in vars(type(spider)).iteritems(): + for key, value in vars(type(spider)).items(): if callable(value) and value.__doc__: bound_method = value.__get__(spider, type(spider)) - request = Request(url='http://scrapy.org', callback=bound_method) + request = self.conman.from_method(bound_method) - # register contract hooks to the request - contracts = Contract.from_method(value) - for contract in contracts: - request = contract.prepare_request(request) - - # discard anything the request might return - cb = request.callback - @wraps(cb) - def wrapper(response): - cb(response) - - request.callback = wrapper - - requests.append(request) + if request: + request.callback = _generate(request.callback) + requests.append(request) return requests diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index fb66291c7..c1f2038b6 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -1,2 +1,76 @@ -from .base import Contract, ContractType -from .default import * +import re +import inspect +from functools import wraps + +from scrapy.http import Request +from scrapy.utils.spider import iterate_spider_output +from scrapy.utils.misc import get_spec +from scrapy.exceptions import ContractFail + +class ContractsManager(object): + registered = {} + + def register(self, contract): + self.registered[contract.name] = contract + + def extract_contracts(self, method): + contracts = [] + for line in method.__doc__.split('\n'): + line = line.strip() + + if line.startswith('@'): + name, args = re.match(r'@(\w+)\s*(.*)', line).groups() + args = re.split(r'\s*\,\s*', args) + + contracts.append(self.registered[name](method, *args)) + + return contracts + + def from_method(self, method): + contracts = self.extract_contracts(method) + if contracts: + # calculate request args + args = get_spec(Request.__init__)[1] + args['callback'] = method + for contract in contracts: + args = contract.adjust_request_args(args) + + # create and prepare request + assert 'url' in args, "Method '%s' does not have an url contract" % method.__name__ + request = Request(**args) + for contract in contracts: + request = contract.prepare_request(request) + + return request + +class Contract(object): + """ Abstract class for contracts """ + + def __init__(self, method, *args): + self.method = method + self.args = args + + def prepare_request(self, request): + cb = request.callback + @wraps(cb) + def wrapper(response): + self.pre_process(response) + output = list(iterate_spider_output(cb(response))) + self.post_process(output) + return output + + request.callback = wrapper + request = self.modify_request(request) + return request + + def adjust_request_args(self, args): + return args + + def modify_request(self, request): + return request + + def pre_process(self, response): + pass + + def post_process(self, output): + pass diff --git a/scrapy/contracts/base.py b/scrapy/contracts/base.py deleted file mode 100644 index f5756be31..000000000 --- a/scrapy/contracts/base.py +++ /dev/null @@ -1,74 +0,0 @@ -import re -from functools import wraps - -from scrapy.utils.spider import iterate_spider_output - -class ContractType(type): - """ Metaclass for contracts - - automatically registers contracts in the root `Contract` class - """ - - def __new__(meta, name, bases, dct): - # only allow single inheritence - assert len(bases) == 1, 'Multiple inheritance is not allowed' - base = bases[0] - - # ascend in inheritence chain - while type(base) not in [type, meta]: - base = type(base) - - # register this as a valid contract - cls = type.__new__(meta, name, bases, dct) - if type(base) != type: - base.registered[cls.name] = cls - return cls - - -class Contract(object): - """ Abstract class for contracts - - keeps a reference of all derived classes in `registered` - """ - - __metaclass__ = ContractType - registered = {} - - def __init__(self, method, *args): - self.method = method - self.args = args - - @classmethod - def from_method(cls, method): - contracts = [] - for line in method.__doc__.split('\n'): - line = line.strip() - - if line.startswith('@'): - name, args = re.match(r'@(\w+)\s*(.*)', line).groups() - args = re.split(r'[\,\s+]', args) - args = filter(lambda x:x, args) - - contracts.append(cls.registered[name](method, *args)) - - return contracts - - def prepare_request(self, request): - cb = request.callback - @wraps(cb) - def wrapper(response): - self.pre_process(response) - output = list(iterate_spider_output(cb(response))) - self.post_process(output) - return output - - request.callback = wrapper - request = self.modify_request(request) - return request - - def modify_request(self, request): - return request - - def pre_process(self, response): - pass - - def post_process(self, output): - pass diff --git a/scrapy/contracts/default.py b/scrapy/contracts/default.py index 77f2e8c75..043a8902a 100644 --- a/scrapy/contracts/default.py +++ b/scrapy/contracts/default.py @@ -1,23 +1,77 @@ from scrapy.item import BaseItem +from scrapy.http import Request +from scrapy.exceptions import ContractFail -from .base import Contract +from . import Contract # contracts class UrlContract(Contract): + """ Contract to set the url of the request (mandatory) + @url http://scrapy.org + """ + name = 'url' - def modify_request(self, request): - return request.replace(url=self.args[0]) + def adjust_request_args(self, args): + args['url'] = self.args[0] + return args -class ReturnsRequestContract(Contract): - name = 'returns_request' +class ReturnsContract(Contract): + """ Contract to check the output of a callback + @returns items, 1 + @returns requests, 1+ + """ + + name = 'returns' + objects = { + 'requests': Request, + 'items': BaseItem, + } + + def __init__(self, *args, **kwargs): + super(ReturnsContract, self).__init__(*args, **kwargs) + + if len(self.args) != 2: + raise ContractError("Returns Contract must have two arguments") + self.obj_name, self.raw_num = self.args + + # validate input + self.obj_type = self.objects[self.obj_name] + + self.modifier = self.raw_num[-1] + if self.modifier in ['+', '-']: + self.num = int(self.raw_num[:-1]) + else: + self.num = int(self.raw_num) + self.modifier = None + + def post_process(self, output): + occurences = 0 + for x in output: + if isinstance(x, self.obj_type): + occurences += 1 + + if self.modifier == '+': + assertion = (occurences >= self.num) + elif self.modifier == '-': + assertion = (occurences <= self.num) + else: + assertion = (occurences == self.num) + + if not assertion: + raise ContractFail("Returned %s %s, expected %s" % \ + (occurences, self.obj_name, self.raw_num)) class ScrapesContract(Contract): + """ Contract to check presence of fields in scraped items + @scrapes page_name, page_body + """ name = 'scrapes' def post_process(self, output): for x in output: if isinstance(x, BaseItem): for arg in self.args: - assert arg in x, '%r field is missing' % arg + if not arg in x: + raise ContractFail('%r field is missing' % arg) diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index 8f29a4c8d..5ecc962ba 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -50,3 +50,6 @@ class ScrapyDeprecationWarning(Warning): """ pass +class ContractFail(Exception): + """Error in constructing contracts for a method""" + pass diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 3baf4cdb7..37a1bd689 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -241,3 +241,10 @@ WEBSERVICE_RESOURCES_BASE = { 'scrapy.contrib.webservice.enginestatus.EngineStatusResource': 1, 'scrapy.contrib.webservice.stats.StatsResource': 1, } + +SPIDER_CONTRACTS = [] +SPIDER_CONTRACTS_BASE = [ + 'scrapy.contracts.default.UrlContract', + 'scrapy.contracts.default.ReturnsContract', + 'scrapy.contracts.default.ScrapesContract', +] diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index fe9b6d058..449923ab1 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -1,6 +1,7 @@ """Helper functions which doesn't fit anywhere else""" import re +import inspect import hashlib from pkgutil import iter_modules @@ -104,3 +105,17 @@ def md5sum(file): m.update(d) return m.hexdigest() +def get_spec(func): + """Returns (args, kwargs) touple for a function + + >>> import re + >>> get_spec(re.match) + (['pattern', 'string'], {'flags': 0}) + """ + spec = inspect.getargspec(func) + defaults = spec.defaults or [] + + firstdefault = len(spec.args) - len(defaults) + args = spec.args[:firstdefault] + kwargs = dict(zip(spec.args[firstdefault:], defaults)) + return args, kwargs From 6f1a5d8db6adf7e84e506321fb47f6636e4ea8aa Mon Sep 17 00:00:00 2001 From: Alex Cepoi Date: Wed, 29 Aug 2012 18:31:10 +0200 Subject: [PATCH 3/6] SEP-017 contracts: various minor changes --- .gitignore | 6 ---- scrapy/commands/check.py | 38 ++++++++++++++------ scrapy/contracts/__init__.py | 47 ++++++++++++++---------- scrapy/contracts/default.py | 56 +++++++++++++++++------------ scrapy/exceptions.py | 4 +-- scrapy/settings/default_settings.py | 12 +++---- scrapy/utils/misc.py | 2 +- 7 files changed, 99 insertions(+), 66 deletions(-) diff --git a/.gitignore b/.gitignore index 39eeda695..f7f30b06f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1,6 @@ *.pyc -*swp -*~ - _trial_temp dropin.cache docs/build *egg-info .tox - -build/ -dist/ diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py index d548e472c..4b4566d11 100644 --- a/scrapy/commands/check.py +++ b/scrapy/commands/check.py @@ -1,3 +1,4 @@ +from collections import defaultdict from functools import wraps from scrapy.conf import settings @@ -7,6 +8,7 @@ from scrapy.contracts import ContractsManager from scrapy.utils import display from scrapy.utils.misc import load_object from scrapy.utils.spider import iterate_spider_output +from scrapy.utils.conf import build_component_list def _generate(cb): """ create a callback which does not return anything """ @@ -19,6 +21,7 @@ def _generate(cb): class Command(ScrapyCommand): requires_project = True + default_settings = {'LOG_ENABLED': False} def syntax(self): return "[options] " @@ -26,27 +29,40 @@ class Command(ScrapyCommand): def short_desc(self): return "Check contracts for given spider" + def add_options(self, parser): + ScrapyCommand.add_options(self, parser) + parser.add_option("-l", "--list", dest="list", action="store_true", \ + help="only list contracts, without checking them") + + def run(self, args, opts): - self.conman = ContractsManager() - # load contracts - contracts = settings['SPIDER_CONTRACTS_BASE'] + \ - settings['SPIDER_CONTRACTS'] + contracts = build_component_list(settings['SPIDER_CONTRACTS_BASE'], + settings['SPIDER_CONTRACTS']) + self.conman = ContractsManager([load_object(c) for c in contracts]) - for contract in contracts: - concls = load_object(contract) - self.conman.register(concls) - - # schedule requests + # contract requests + contract_reqs = defaultdict(list) self.crawler.engine.has_capacity = lambda: True for spider in args or self.crawler.spiders.list(): spider = self.crawler.spiders.create(spider) requests = self.get_requests(spider) - self.crawler.crawl(spider, requests) + + if opts.list: + for req in requests: + contract_reqs[spider.name].append(req.callback.__name__) + else: + self.crawler.crawl(spider, requests) # start checks - self.crawler.start() + if opts.list: + for spider, methods in sorted(contract_reqs.iteritems()): + print spider + for method in sorted(methods): + print ' * %s' % method + else: + self.crawler.start() def get_requests(self, spider): requests = [] diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index c1f2038b6..51f7d94c6 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -8,10 +8,11 @@ from scrapy.utils.misc import get_spec from scrapy.exceptions import ContractFail class ContractsManager(object): - registered = {} + contracts = {} - def register(self, contract): - self.registered[contract.name] = contract + def __init__(self, contracts): + for contract in contracts: + self.contracts[contract.name] = contract def extract_contracts(self, method): contracts = [] @@ -20,9 +21,9 @@ class ContractsManager(object): if line.startswith('@'): name, args = re.match(r'@(\w+)\s*(.*)', line).groups() - args = re.split(r'\s*\,\s*', args) + args = re.split(r'\s+', args) - contracts.append(self.registered[name](method, *args)) + contracts.append(self.contracts[name](method, *args)) return contracts @@ -30,18 +31,23 @@ class ContractsManager(object): contracts = self.extract_contracts(method) if contracts: # calculate request args - args = get_spec(Request.__init__)[1] - args['callback'] = method + args, kwargs = get_spec(Request.__init__) + kwargs['callback'] = method for contract in contracts: - args = contract.adjust_request_args(args) + kwargs = contract.adjust_request_args(kwargs) # create and prepare request - assert 'url' in args, "Method '%s' does not have an url contract" % method.__name__ - request = Request(**args) - for contract in contracts: - request = contract.prepare_request(request) + args.remove('self') + if set(args).issubset(set(kwargs)): + request = Request(**kwargs) - return request + # execute pre and post hooks in order + for contract in reversed(contracts): + request = contract.add_pre_hook(request) + for contract in contracts: + request = contract.add_post_hook(request) + + return request class Contract(object): """ Abstract class for contracts """ @@ -50,25 +56,30 @@ class Contract(object): self.method = method self.args = args - def prepare_request(self, request): + def add_pre_hook(self, request): cb = request.callback @wraps(cb) def wrapper(response): self.pre_process(response) + return list(iterate_spider_output(cb(response))) + + request.callback = wrapper + return request + + def add_post_hook(self, request): + cb = request.callback + @wraps(cb) + def wrapper(response): output = list(iterate_spider_output(cb(response))) self.post_process(output) return output request.callback = wrapper - request = self.modify_request(request) return request def adjust_request_args(self, args): return args - def modify_request(self, request): - return request - def pre_process(self, response): pass diff --git a/scrapy/contracts/default.py b/scrapy/contracts/default.py index 043a8902a..73a1447f0 100644 --- a/scrapy/contracts/default.py +++ b/scrapy/contracts/default.py @@ -17,34 +17,44 @@ class UrlContract(Contract): args['url'] = self.args[0] return args + class ReturnsContract(Contract): """ Contract to check the output of a callback - @returns items, 1 - @returns requests, 1+ + + general form: + @returns request(s)/item(s) [min=1 [max]] + + e.g.: + @returns request + @returns request 2 + @returns request 2 10 + @returns request 0 10 """ name = 'returns' objects = { + 'request': Request, 'requests': Request, + 'item': BaseItem, 'items': BaseItem, } def __init__(self, *args, **kwargs): super(ReturnsContract, self).__init__(*args, **kwargs) - if len(self.args) != 2: - raise ContractError("Returns Contract must have two arguments") - self.obj_name, self.raw_num = self.args - - # validate input + assert len(self.args) in [1, 2, 3] + self.obj_name = self.args[0] or None self.obj_type = self.objects[self.obj_name] - self.modifier = self.raw_num[-1] - if self.modifier in ['+', '-']: - self.num = int(self.raw_num[:-1]) - else: - self.num = int(self.raw_num) - self.modifier = None + try: + self.min_bound = int(self.args[1]) + except IndexError: + self.min_bound = 1 + + try: + self.max_bound = int(self.args[2]) + except IndexError: + self.max_bound = float('inf') def post_process(self, output): occurences = 0 @@ -52,21 +62,23 @@ class ReturnsContract(Contract): if isinstance(x, self.obj_type): occurences += 1 - if self.modifier == '+': - assertion = (occurences >= self.num) - elif self.modifier == '-': - assertion = (occurences <= self.num) - else: - assertion = (occurences == self.num) + assertion = (self.min_bound <= occurences <= self.max_bound) if not assertion: + if self.min_bound == self.max_bound: + expected = self.min_bound + else: + expected = '%s..%s' % (self.min_bound, self.max_bound) + raise ContractFail("Returned %s %s, expected %s" % \ - (occurences, self.obj_name, self.raw_num)) + (occurences, self.obj_name, expected)) + class ScrapesContract(Contract): """ Contract to check presence of fields in scraped items - @scrapes page_name, page_body + @scrapes page_name page_body """ + name = 'scrapes' def post_process(self, output): @@ -74,4 +86,4 @@ class ScrapesContract(Contract): if isinstance(x, BaseItem): for arg in self.args: if not arg in x: - raise ContractFail('%r field is missing' % arg) + raise ContractFail("'%s' field is missing" % arg) diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index 5ecc962ba..c22ba0204 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -50,6 +50,6 @@ class ScrapyDeprecationWarning(Warning): """ pass -class ContractFail(Exception): - """Error in constructing contracts for a method""" +class ContractFail(AssertionError): + """Error raised in case of a failing contract""" pass diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 37a1bd689..a8882a78d 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -242,9 +242,9 @@ WEBSERVICE_RESOURCES_BASE = { 'scrapy.contrib.webservice.stats.StatsResource': 1, } -SPIDER_CONTRACTS = [] -SPIDER_CONTRACTS_BASE = [ - 'scrapy.contracts.default.UrlContract', - 'scrapy.contracts.default.ReturnsContract', - 'scrapy.contracts.default.ScrapesContract', -] +SPIDER_CONTRACTS = {} +SPIDER_CONTRACTS_BASE = { + 'scrapy.contracts.default.UrlContract' : 1, + 'scrapy.contracts.default.ReturnsContract': 2, + 'scrapy.contracts.default.ScrapesContract': 3, +} diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index 449923ab1..62fe4c3c1 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -106,7 +106,7 @@ def md5sum(file): return m.hexdigest() def get_spec(func): - """Returns (args, kwargs) touple for a function + """Returns (args, kwargs) tuple for a function >>> import re >>> get_spec(re.match) From bf8dc61fb714e78c2ff734decc33a2d00e60d92c Mon Sep 17 00:00:00 2001 From: Alex Cepoi Date: Mon, 10 Sep 2012 23:17:27 +0200 Subject: [PATCH 4/6] SEP-017 contracts: pretty-printing and docs --- docs/index.rst | 4 ++ docs/topics/commands.rst | 28 +++++++++ docs/topics/settings.rst | 24 ++++++++ docs/topics/testing.rst | 113 +++++++++++++++++++++++++++++++++++ scrapy/contracts/__init__.py | 8 ++- scrapy/exceptions.py | 5 +- 6 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 docs/topics/testing.rst diff --git a/docs/index.rst b/docs/index.rst index 317ccb2e1..4fd015faf 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -129,6 +129,7 @@ Solving specific problems faq topics/debug + topics/testing topics/firefox topics/firebug topics/leaks @@ -143,6 +144,9 @@ Solving specific problems :doc:`topics/debug` Learn how to debug common problems of your scrapy spider. +:doc:`topics/testing` + Learn how to use contracts for testing your spiders. + :doc:`topics/firefox` Learn how to scrape with Firefox and some useful add-ons. diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index bac238fb8..5ca53d273 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -142,6 +142,7 @@ Global commands: Project-only commands: * :command:`crawl` +* :command:`check` * :command:`list` * :command:`edit` * :command:`parse` @@ -221,6 +222,33 @@ Usage examples:: [ ... myspider starts crawling ... ] +.. command:: check + +check +----- + +* Syntax: ``scrapy check [-l] `` +* Requires project: *yes* + +Run contract checks. + +Usage examples:: + + $ scrapy check -l + first_spider + * parse + * parse_item + second_spider + * parse + * parse_item + + $ scrapy check + [FAILED] first_spider:parse_item + >>> 'RetailPricex' field is missing + + [FAILED] first_spider:parse + >>> Returned 92 requests, expected 0..4 + .. command:: server server diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 682206db8..c2bc8d584 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -832,6 +832,30 @@ The scheduler to use for crawling. .. setting:: SPIDER_MIDDLEWARES + +SPIDER_CONTRACTS +---------------- + +Default:: ``{}`` + +A dict containing the scrapy contracts enabled in your project, used for +testing spiders. For more info see :ref:`topics-testing`. + +SPIDER_CONTRACTS_BASE +--------------------- + +Default:: + + { + 'scrapy.contracts.default.UrlContract' : 1, + 'scrapy.contracts.default.ReturnsContract': 2, + 'scrapy.contracts.default.ScrapesContract': 3, + } + +A dict containing the scrapy contracts enabled by default in Scrapy. You should +never modify this setting in your project, modify :setting:`SPIDER_CONTRACTS` +instead. For more info see :ref:`topics-testing`. + SPIDER_MIDDLEWARES ------------------ diff --git a/docs/topics/testing.rst b/docs/topics/testing.rst new file mode 100644 index 000000000..b24e62dce --- /dev/null +++ b/docs/topics/testing.rst @@ -0,0 +1,113 @@ +.. _topics-testing: + +=============== +Testing Spiders +=============== + +Testing spiders can get particularly annoying and while nothing prevents you +from writing unit tests the task gets cumbersome quickly. Scrapy offers an +integrated way of testing your spiders by the means of contracts. + +This allows you to test each callback of your spider by hardcoding a sample url +and check various constraints for how the callback processes the response. Each +contract is prefixed with an ``@`` and included in the docstring. See the +following example:: + + def parse(self, response): + """ This function parses a sample response. Some contracts are mingled + with this docstring. + + @url http://www.amazon.com/s?field-keywords=selfish+gene + @returns items 1 16 + @returns requests 0 0 + @scrapes Title Author Year Price + """ + +This callback is tested using three built-in contracts: + +.. module:: scrapy.contracts.default + +.. class:: UrlContract + + This contract (``@url``) sets the sample url used when checking other + contract conditions for this spider. This contract is mandatory. All + callbacks lacking this contract are ignored when running the checks:: + + @url url + +.. class:: ReturnsContract + + This contract (``@returns``) sets lower and upper bounds for the items and + requests returned by the spider. The upper bound is optional:: + + @returns item(s)|request(s) [min [max]] + +.. class:: ScrapesContract + + This contract (``@scrapes``) checks that all the items returned by the + callback have the specified fields:: + + @scrapes field_1 field_2 ... + +Use the :command:`check` command to run the contract checks. + +Custom Contracts +================ + +If you find you need more power than the built-in scrapy contracts you can +create and load your own contracts in the project by using the +:setting:`SPIDER_CONTRACTS` setting:: + + SPIDER_CONTRACTS = { + 'myproject.contracts.ResponseCheck': 10, + 'myproject.contracts.ItemValidate': 10, + } + +Each contract must inherit from :class:`scrapy.contracts.Contract` and can +override three methods: + +.. module:: scrapy.contracts + +.. class:: Contract(method, \*args) + + :param method: callback function to which the contract is associated + :type method: function + + :param args: list of arguments passed into the docstring (whitespace + separated) + :type args: list + + .. method:: Contract.adjust_request_args(args) + + This receives a ``dict`` as an argument containing default arguments + for :class:`~scrapy.http.Request` object. Must return the same or a + modified version of it. + + .. method:: Contract.pre_process(response) + + This allows hooking in various checks on the response received from the + sample request, before it's being passed to the callback. + + .. method:: Contract.post_process(output) + + This allows processing the output of the callback. Iterators are + converted listified before being passed to this hook. + +Here is a demo contract which checks the presence of a custom header in the +response received. Raise :class:`scrapy.exceptions.ContractFail` in order to +get the failures pretty printed:: + + from scrapy.contracts import Contract + from scrapy.exceptions import ContractFail + + class HasHeaderContract(Contract): + """ Demo contract which checks the presence of a custom header + @has_header X-CustomHeader + """ + + name = 'has_header' + + def pre_process(self, response): + for header in self.args: + if header not in response.headers: + raise ContractFail('X-CustomHeader not present') diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index 51f7d94c6..26cde61e3 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -60,7 +60,9 @@ class Contract(object): cb = request.callback @wraps(cb) def wrapper(response): - self.pre_process(response) + try: self.pre_process(response) + except ContractFail as e: + print e.format(self.method) return list(iterate_spider_output(cb(response))) request.callback = wrapper @@ -71,7 +73,9 @@ class Contract(object): @wraps(cb) def wrapper(response): output = list(iterate_spider_output(cb(response))) - self.post_process(output) + try: self.post_process(output) + except ContractFail as e: + print e.format(self.method) return output request.callback = wrapper diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index c22ba0204..f41403bb4 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -52,4 +52,7 @@ class ScrapyDeprecationWarning(Warning): class ContractFail(AssertionError): """Error raised in case of a failing contract""" - pass + + def format(self, method): + return '[FAILED] %s:%s\n>>> %s\n' % \ + (method.im_class.name, method.__name__, self) From 11d29c70059ee82e5608aaf0c958be19ece6e97a Mon Sep 17 00:00:00 2001 From: Alex Cepoi Date: Fri, 21 Sep 2012 00:12:46 +0200 Subject: [PATCH 5/6] SEP-017 contracts: add tests and minor improvements --- docs/index.rst | 4 +- docs/topics/{testing.rst => contracts.rst} | 8 +- scrapy/commands/check.py | 15 ++- scrapy/contracts/__init__.py | 33 ++++-- scrapy/tests/test_contracts.py | 124 +++++++++++++++++++++ scrapy/utils/misc.py | 15 --- scrapy/utils/python.py | 36 ++++++ 7 files changed, 195 insertions(+), 40 deletions(-) rename docs/topics/{testing.rst => contracts.rst} (97%) create mode 100644 scrapy/tests/test_contracts.py diff --git a/docs/index.rst b/docs/index.rst index 4fd015faf..3ebfbb577 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -129,7 +129,7 @@ Solving specific problems faq topics/debug - topics/testing + topics/contracts topics/firefox topics/firebug topics/leaks @@ -144,7 +144,7 @@ Solving specific problems :doc:`topics/debug` Learn how to debug common problems of your scrapy spider. -:doc:`topics/testing` +:doc:`topics/contracts` Learn how to use contracts for testing your spiders. :doc:`topics/firefox` diff --git a/docs/topics/testing.rst b/docs/topics/contracts.rst similarity index 97% rename from docs/topics/testing.rst rename to docs/topics/contracts.rst index b24e62dce..8e98bf749 100644 --- a/docs/topics/testing.rst +++ b/docs/topics/contracts.rst @@ -1,8 +1,8 @@ -.. _topics-testing: +.. _topics-contracts: -=============== -Testing Spiders -=============== +================= +Spiders Contracts +================= Testing spiders can get particularly annoying and while nothing prevents you from writing unit tests the task gets cumbersome quickly. Scrapy offers an diff --git a/scrapy/commands/check.py b/scrapy/commands/check.py index 4b4566d11..13190e025 100644 --- a/scrapy/commands/check.py +++ b/scrapy/commands/check.py @@ -1,24 +1,22 @@ from collections import defaultdict from functools import wraps -from scrapy.conf import settings from scrapy.command import ScrapyCommand -from scrapy.http import Request from scrapy.contracts import ContractsManager -from scrapy.utils import display from scrapy.utils.misc import load_object from scrapy.utils.spider import iterate_spider_output from scrapy.utils.conf import build_component_list + def _generate(cb): """ create a callback which does not return anything """ @wraps(cb) def wrapper(response): output = cb(response) output = list(iterate_spider_output(output)) - # display.pprint(output) return wrapper + class Command(ScrapyCommand): requires_project = True default_settings = {'LOG_ENABLED': False} @@ -31,14 +29,15 @@ class Command(ScrapyCommand): def add_options(self, parser): ScrapyCommand.add_options(self, parser) - parser.add_option("-l", "--list", dest="list", action="store_true", \ + parser.add_option("-l", "--list", dest="list", action="store_true", help="only list contracts, without checking them") - def run(self, args, opts): # load contracts - contracts = build_component_list(settings['SPIDER_CONTRACTS_BASE'], - settings['SPIDER_CONTRACTS']) + contracts = build_component_list( + self.settings['SPIDER_CONTRACTS_BASE'], + self.settings['SPIDER_CONTRACTS'], + ) self.conman = ContractsManager([load_object(c) for c in contracts]) # contract requests diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index 26cde61e3..fac226822 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -1,12 +1,12 @@ import re -import inspect from functools import wraps from scrapy.http import Request from scrapy.utils.spider import iterate_spider_output -from scrapy.utils.misc import get_spec +from scrapy.utils.python import get_spec from scrapy.exceptions import ContractFail + class ContractsManager(object): contracts = {} @@ -27,7 +27,7 @@ class ContractsManager(object): return contracts - def from_method(self, method): + def from_method(self, method, fail=False): contracts = self.extract_contracts(method) if contracts: # calculate request args @@ -43,12 +43,13 @@ class ContractsManager(object): # execute pre and post hooks in order for contract in reversed(contracts): - request = contract.add_pre_hook(request) + request = contract.add_pre_hook(request, fail) for contract in contracts: - request = contract.add_post_hook(request) + request = contract.add_post_hook(request, fail) return request + class Contract(object): """ Abstract class for contracts """ @@ -56,26 +57,36 @@ class Contract(object): self.method = method self.args = args - def add_pre_hook(self, request): + def add_pre_hook(self, request, fail=False): cb = request.callback + @wraps(cb) def wrapper(response): - try: self.pre_process(response) + try: + self.pre_process(response) except ContractFail as e: - print e.format(self.method) + if fail: + raise + else: + print e.format(self.method) return list(iterate_spider_output(cb(response))) request.callback = wrapper return request - def add_post_hook(self, request): + def add_post_hook(self, request, fail=False): cb = request.callback + @wraps(cb) def wrapper(response): output = list(iterate_spider_output(cb(response))) - try: self.post_process(output) + try: + self.post_process(output) except ContractFail as e: - print e.format(self.method) + if fail: + raise + else: + print e.format(self.method) return output request.callback = wrapper diff --git a/scrapy/tests/test_contracts.py b/scrapy/tests/test_contracts.py new file mode 100644 index 000000000..b35e57519 --- /dev/null +++ b/scrapy/tests/test_contracts.py @@ -0,0 +1,124 @@ +from twisted.trial import unittest + +from scrapy.spider import BaseSpider +from scrapy.http import Request +from scrapy.item import Item, Field +from scrapy.exceptions import ContractFail +from scrapy.contracts import ContractsManager +from scrapy.contracts.default import ( + UrlContract, + ReturnsContract, + ScrapesContract, +) + + +class TestItem(Item): + name = Field() + url = Field() + + +class ResponseMock(object): + url = 'http://scrapy.org' + + +class TestSpider(BaseSpider): + name = 'demo_spider' + + def returns_request(self, response): + """ method which returns request + @url http://scrapy.org + @returns requests 1 + """ + return Request('http://scrapy.org', callback=self.returns_item) + + def returns_item(self, response): + """ method which returns item + @url http://scrapy.org + @returns items 1 1 + """ + return TestItem(url=response.url) + + def returns_fail(self, response): + """ method which returns item + @url http://scrapy.org + @returns items 0 0 + """ + return TestItem(url=response.url) + + def scrapes_item_ok(self, response): + """ returns item with name and url + @url http://scrapy.org + @returns items 1 1 + @scrapes name url + """ + return TestItem(name='test', url=response.url) + + def scrapes_item_fail(self, response): + """ returns item with no name + @url http://scrapy.org + @returns items 1 1 + @scrapes name url + """ + return TestItem(url=response.url) + + def parse_no_url(self, response): + """ method with no url + @returns items 1 1 + """ + pass + + +class ContractsManagerTest(unittest.TestCase): + contracts = [UrlContract, ReturnsContract, ScrapesContract] + + def test_contracts(self): + conman = ContractsManager(self.contracts) + + # extract contracts correctly + contracts = conman.extract_contracts(TestSpider.returns_request) + self.assertEqual(len(contracts), 2) + self.assertEqual(frozenset(map(type, contracts)), + frozenset([UrlContract, ReturnsContract])) + + # returns request for valid method + request = conman.from_method(TestSpider.returns_request) + self.assertIsNotNone(request) + + # no request for missing url + request = conman.from_method(TestSpider.parse_no_url) + self.assertIsNone(request) + + def test_returns(self): + conman = ContractsManager(self.contracts) + + spider = TestSpider() + response = ResponseMock() + + # returns_item + request = conman.from_method(spider.returns_item, fail=True) + output = request.callback(response) + self.assertEqual(map(type, output), [TestItem]) + + # returns_request + request = conman.from_method(spider.returns_request, fail=True) + output = request.callback(response) + self.assertEqual(map(type, output), [Request]) + + # returns_fail + request = conman.from_method(spider.returns_fail, fail=True) + self.assertRaises(ContractFail, request.callback, response) + + def test_scrapes(self): + conman = ContractsManager(self.contracts) + + spider = TestSpider() + response = ResponseMock() + + # scrapes_item_ok + request = conman.from_method(spider.scrapes_item_ok, fail=True) + output = request.callback(response) + self.assertEqual(map(type, output), [TestItem]) + + # scrapes_item_fail + request = conman.from_method(spider.scrapes_item_fail, fail=True) + self.assertRaises(ContractFail, request.callback, response) diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index 62fe4c3c1..99e5317f2 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -104,18 +104,3 @@ def md5sum(file): break m.update(d) return m.hexdigest() - -def get_spec(func): - """Returns (args, kwargs) tuple for a function - - >>> import re - >>> get_spec(re.match) - (['pattern', 'string'], {'flags': 0}) - """ - spec = inspect.getargspec(func) - defaults = spec.defaults or [] - - firstdefault = len(spec.args) - len(defaults) - args = spec.args[:firstdefault] - kwargs = dict(zip(spec.args[firstdefault:], defaults)) - return args, kwargs diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 82c953abc..64117d1d1 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -158,6 +158,42 @@ def get_func_args(func): raise TypeError('%s is not callable' % type(func)) return func_args +def get_spec(func): + """Returns (args, kwargs) tuple for a function + >>> import re + >>> get_spec(re.match) + (['pattern', 'string'], {'flags': 0}) + + >>> class Test(object): + ... def __call__(self, val): + ... pass + ... def method(self, val, flags=0): + ... pass + + >>> get_spec(Test) + (['self', 'val'], {}) + + >>> get_spec(Test.method) + (['self', 'val'], {'flags': 0}) + + >>> get_spec(Test().method) + (['self', 'val'], {'flags': 0}) + """ + + if inspect.isfunction(func) or inspect.ismethod(func): + spec = inspect.getargspec(func) + elif hasattr(func, '__call__'): + spec = inspect.getargspec(func.__call__) + else: + raise TypeError('%s is not callable' % type(func)) + + defaults = spec.defaults or [] + + firstdefault = len(spec.args) - len(defaults) + args = spec.args[:firstdefault] + kwargs = dict(zip(spec.args[firstdefault:], defaults)) + return args, kwargs + def equal_attributes(obj1, obj2, attributes): """Compare two objects attributes""" # not attributes given return False by default From 73e6bc1b106861db4d090dce6789ba08854107d4 Mon Sep 17 00:00:00 2001 From: Alex Cepoi Date: Fri, 21 Sep 2012 00:54:11 +0200 Subject: [PATCH 6/6] remove unused import --- scrapy/utils/misc.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index 99e5317f2..543892d7e 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -1,7 +1,6 @@ """Helper functions which doesn't fit anywhere else""" import re -import inspect import hashlib from pkgutil import iter_modules