From f56079f6c71a77c1f70510cf291cd808617933cd Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 5 Dec 2018 10:02:42 +0000 Subject: [PATCH] Test cleanups PEP8 fixes no need to close implicitly do not use pytest need to put it into class remove round-robin queue additional check for empty queue use pytest tmpdir fixture --- scrapy/pqueues.py | 50 ++++------------ tests/test_scheduler.py | 128 ++++++++++++---------------------------- 2 files changed, 50 insertions(+), 128 deletions(-) diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 75073b7a4..287a8de35 100644 --- a/scrapy/pqueues.py +++ b/scrapy/pqueues.py @@ -1,4 +1,3 @@ -from collections import deque import hashlib import logging from six import text_type @@ -71,16 +70,17 @@ class PrioritySlot: self.slot = slot def __hash__(self): - return hash((self.priority, self.slot)) + return hash((self.priority, self.slot)) def __eq__(self, other): - return (self.priority, self.slot) == (other.priority, other.slot) + return (self.priority, self.slot) == (other.priority, other.slot) def __lt__(self, other): - return (self.priority, self.slot) < (other.priority, other.slot) + return (self.priority, self.slot) < (other.priority, other.slot) def __str__(self): - return '_'.join([text_type(self.priority), _pathable(text_type(self.slot))]) + return '_'.join([text_type(self.priority), + _pathable(text_type(self.slot))]) class PriorityAsTupleQueue(PriorityQueue): @@ -135,9 +135,10 @@ class SlotBasedPriorityQueue(object): slot = _scheduler_slot(request) is_new = False if slot not in self.pqueues: - is_new = True self.pqueues[slot] = PriorityAsTupleQueue(self.qfactory) - self.pqueues[slot].push(request, PrioritySlot(priority=priority, slot=slot)) + queue = self.pqueues[slot] + is_new = queue.is_empty() + queue.push(request, PrioritySlot(priority=priority, slot=slot)) return slot, is_new def close(self): @@ -152,36 +153,6 @@ class SlotBasedPriorityQueue(object): return sum(len(x) for x in self.pqueues.values()) if self.pqueues else 0 -class RoundRobinPriorityQueue(SlotBasedPriorityQueue): - - def __init__(self, qfactory, startprios={}): - super(RoundRobinPriorityQueue, self).__init__(qfactory, startprios) - self._slots = deque() - for slot in self.pqueues: - self._slots.append(slot) - - def push(self, request, priority): - slot, is_new = self.push_slot(request, priority) - if is_new: - self._slots.append(slot) - - def pop(self): - if not self._slots: - return - - slot = self._slots.popleft() - request, is_empty = self.pop_slot(slot) - - if not is_empty: - self._slots.append(slot) - - return request - - def close(self): - self._slots.clear() - return super(RoundRobinPriorityQueue, self).close() - - class DownloaderAwarePriorityQueue(SlotBasedPriorityQueue): _DOWNLOADER_AWARE_PQ_ID = 'DOWNLOADER_AWARE_PQ_ID' @@ -191,7 +162,8 @@ class DownloaderAwarePriorityQueue(SlotBasedPriorityQueue): return cls(crawler, qfactory, startprios) def __init__(self, crawler, qfactory, startprios={}): - super(DownloaderAwarePriorityQueue, self).__init__(qfactory, startprios) + super(DownloaderAwarePriorityQueue, self).__init__(qfactory, + startprios) self._slots = {slot: 0 for slot in self.pqueues} crawler.signals.connect(self.on_response_download, signal=response_downloaded) @@ -208,7 +180,7 @@ class DownloaderAwarePriorityQueue(SlotBasedPriorityQueue): return request.meta.get(self._DOWNLOADER_AWARE_PQ_ID, None) == id(self) def pop(self): - slots = [(d, s) for s,d in self._slots.items() if s in self.pqueues] + slots = [(d, s) for s, d in self._slots.items() if s in self.pqueues] if not slots: return diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index fd86e8d8c..e1cf5842d 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -1,4 +1,3 @@ -import contextlib import shutil import tempfile import unittest @@ -10,15 +9,18 @@ from scrapy.pqueues import _scheduler_slot_read, _scheduler_slot_write from scrapy.signals import request_reached_downloader, response_downloaded from scrapy.spiders import Spider + class MockCrawler(Crawler): def __init__(self, priority_queue_cls, jobdir): - settings = dict(LOG_UNSERIALIZABLE_REQUESTS=False, - SCHEDULER_DISK_QUEUE='scrapy.squeues.PickleLifoDiskQueue', - SCHEDULER_MEMORY_QUEUE='scrapy.squeues.LifoMemoryQueue', - SCHEDULER_PRIORITY_QUEUE=priority_queue_cls, - JOBDIR=jobdir, - DUPEFILTER_CLASS='scrapy.dupefilters.BaseDupeFilter') + settings = dict( + LOG_UNSERIALIZABLE_REQUESTS=False, + SCHEDULER_DISK_QUEUE='scrapy.squeues.PickleLifoDiskQueue', + SCHEDULER_MEMORY_QUEUE='scrapy.squeues.LifoMemoryQueue', + SCHEDULER_PRIORITY_QUEUE=priority_queue_cls, + JOBDIR=jobdir, + DUPEFILTER_CLASS='scrapy.dupefilters.BaseDupeFilter' + ) super(MockCrawler, self).__init__(Spider, settings) @@ -82,7 +84,8 @@ class BaseSchedulerInMemoryTester(SchedulerHandler): while self.scheduler.has_pending_requests(): priorities.append(self.scheduler.next_request().priority) - self.assertEqual(priorities, sorted([x[1] for x in _PRIORITIES], key=lambda x: -x)) + self.assertEqual(priorities, + sorted([x[1] for x in _PRIORITIES], key=lambda x: -x)) class BaseSchedulerOnDiskTester(SchedulerHandler): @@ -134,7 +137,8 @@ class BaseSchedulerOnDiskTester(SchedulerHandler): while self.scheduler.has_pending_requests(): priorities.append(self.scheduler.next_request().priority) - self.assertEqual(priorities, sorted([x[1] for x in _PRIORITIES], key=lambda x: -x)) + self.assertEqual(priorities, + sorted([x[1] for x in _PRIORITIES], key=lambda x: -x)) class TestSchedulerInMemory(BaseSchedulerInMemoryTester, unittest.TestCase): @@ -153,75 +157,15 @@ _SLOTS = [("http://foo.com/a", 'a'), ("http://foo.com/f", 'c')] -class TestSchedulerWithRoundRobinInMemory(BaseSchedulerInMemoryTester, unittest.TestCase): - priority_queue_cls = 'scrapy.pqueues.RoundRobinPriorityQueue' +class TestMigration(unittest.TestCase): - def test_round_robin(self): - for url, slot in _SLOTS: - request = Request(url) - _scheduler_slot_write(request, slot) - self.scheduler.enqueue_request(request) + def setUp(self): + self.tmpdir = tempfile.mkdtemp() - slots = list() - while self.scheduler.has_pending_requests(): - slots.append(_scheduler_slot_read(self.scheduler.next_request())) + def tearDown(self): + shutil.rmtree(self.tmpdir) - for i in range(0, len(_SLOTS), 2): - self.assertNotEqual(slots[i], slots[i+1]) - - def test_is_meta_set(self): - url = "http://foo.com/a" - request = Request(url) - if _scheduler_slot_read(request): - _scheduler_slot_write(request, None) - self.scheduler.enqueue_request(request) - self.assertIsNotNone(_scheduler_slot_read(request, None), None) - - -class TestSchedulerWithRoundRobinOnDisk(BaseSchedulerOnDiskTester, unittest.TestCase): - priority_queue_cls = 'scrapy.pqueues.RoundRobinPriorityQueue' - - def test_round_robin(self): - for url, slot in _SLOTS: - request = Request(url) - _scheduler_slot_write(request, slot) - self.scheduler.enqueue_request(request) - - self.close_scheduler() - self.create_scheduler() - - slots = list() - while self.scheduler.has_pending_requests(): - slots.append(_scheduler_slot_read(self.scheduler.next_request())) - - for i in range(0, len(_SLOTS), 2): - self.assertNotEqual(slots[i], slots[i+1]) - - def test_is_meta_set(self): - url = "http://foo.com/a" - request = Request(url) - if _scheduler_slot_read(request): - _scheduler_slot_write(request, None) - self.scheduler.enqueue_request(request) - - self.close_scheduler() - self.create_scheduler() - - self.assertIsNotNone(_scheduler_slot_read(request, None), None) - - -@contextlib.contextmanager -def mkdtemp(): - dir = tempfile.mkdtemp() - try: - yield dir - finally: - shutil.rmtree(dir) - - -def _migration(): - - with mkdtemp() as tmp_dir: + def _migration(self, tmp_dir): prev_scheduler_handler = SchedulerHandler() prev_scheduler_handler.priority_queue_cls = 'queuelib.PriorityQueue' prev_scheduler_handler.jobdir = tmp_dir @@ -232,18 +176,18 @@ def _migration(): prev_scheduler_handler.close_scheduler() next_scheduler_handler = SchedulerHandler() - next_scheduler_handler.priority_queue_cls = 'scrapy.pqueues.RoundRobinPriorityQueue' + next_scheduler_handler.priority_queue_cls = 'scrapy.pqueues.DownloaderAwarePriorityQueue' next_scheduler_handler.jobdir = tmp_dir next_scheduler_handler.create_scheduler() - -class TestMigration(unittest.TestCase): def test_migration(self): - self.assertRaises(ValueError, _migration) + with self.assertRaises(ValueError): + self._migration(self.tmpdir) -class TestSchedulerWithDownloaderAwareInMemory(BaseSchedulerInMemoryTester, unittest.TestCase): +class TestSchedulerWithDownloaderAwareInMemory(BaseSchedulerInMemoryTester, + unittest.TestCase): priority_queue_cls = 'scrapy.pqueues.DownloaderAwarePriorityQueue' def test_logic(self): @@ -266,10 +210,12 @@ class TestSchedulerWithDownloaderAwareInMemory(BaseSchedulerInMemoryTester, unit self.assertEqual(len(slots), len(_SLOTS)) for request in requests: - self.mock_crawler.signals.send_catch_log(signal=response_downloaded, - request=request, - response=None, - spider=self.spider) + self.mock_crawler.signals.send_catch_log( + signal=response_downloaded, + request=request, + response=None, + spider=self.spider + ) unique_slots = len(set(s for _, s in _SLOTS)) for i in range(0, len(_SLOTS), unique_slots): @@ -277,8 +223,10 @@ class TestSchedulerWithDownloaderAwareInMemory(BaseSchedulerInMemoryTester, unit self.assertEqual(len(part), len(set(part))) -class TestSchedulerWithDownloaderAwareOnDisk(BaseSchedulerOnDiskTester, unittest.TestCase): +class TestSchedulerWithDownloaderAwareOnDisk(BaseSchedulerOnDiskTester, + unittest.TestCase): priority_queue_cls = 'scrapy.pqueues.DownloaderAwarePriorityQueue' + def test_logic(self): for url, slot in _SLOTS: request = Request(url) @@ -304,10 +252,12 @@ class TestSchedulerWithDownloaderAwareOnDisk(BaseSchedulerOnDiskTester, unittest self.assertEqual(len(slots), len(_SLOTS)) for request in requests: - self.mock_crawler.signals.send_catch_log(signal=response_downloaded, - request=request, - response=None, - spider=self.spider) + self.mock_crawler.signals.send_catch_log( + signal=response_downloaded, + request=request, + response=None, + spider=self.spider + ) unique_slots = len(set(s for _, s in _SLOTS)) for i in range(0, len(_SLOTS), unique_slots):