From 7a51d370f3a59bcfe118f3ba079c5ea6eda5af75 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 17:16:27 +0100 Subject: [PATCH] Regex-based guess_scheme() + refactor tests --- scrapy/commands/shell.py | 28 ++++++---- tests/test_command_shell.py | 102 +++++++++++++++++------------------- 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 25ceb5f99..5201feb42 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -3,7 +3,7 @@ Scrapy Shell See documentation in docs/topics/shell.rst """ - +import re from six.moves.urllib.parse import urlparse from threading import Thread from w3lib.url import any_to_uri @@ -21,16 +21,22 @@ def guess_scheme(url): otherwise returns an HTTP URL """ parts = urlparse(url) - if not parts.scheme: - if "." not in parts.path.split("/", 1)[0]: - url = any_to_uri(url) - - for pattern in ["/", "./", "../"]: - if url.startswith(pattern): - url = any_to_uri(url) - break - url = add_http_if_no_scheme(url) - return url + if parts.scheme: + return url + # Note: this does not match Windows filepath + if re.match(r'''^ # start with... + ( + \. # ...a single dot, + ( + \. | [^/\.]+ # optionally followed by + )? # either a second dot or some characters + )? # optional match of ".", ".." or ".blabla" + / # at least one "/" for a file path, + . # and something after the "/" + ''', parts.path, flags=re.VERBOSE): + return any_to_uri(url) + else: + return add_http_if_no_scheme(url) class Command(ScrapyCommand): diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 37c6b8c90..a61d520fa 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -11,67 +11,61 @@ from tests import tests_datadir class ShellURLTest(unittest.TestCase): + pass - def test_file_uri_relative001(self): - # FIXME: 'index.html' is interpreted as a domain name - # is this correct? - url = guess_scheme('index.html') - assert url.startswith('http://') +def create_guess_scheme_t(args): + def do_expected(self): + url = guess_scheme(args[0]) + assert url.startswith(args[1]), \ + 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( + args[0], url, args[1]) + return do_expected - def test_file_uri_relative002(self): - url = guess_scheme('./index.html') - assert url.startswith('file://') +def create_skipped_scheme_t(args): + def do_expected(self): + raise unittest.SkipTest(args[2]) + url = guess_scheme(args[0]) + assert url.startswith(args[1]) + return do_expected - def test_file_uri_relative003(self): - url = guess_scheme('../data/index.html') - assert url.startswith('file://') +for k, args in enumerate ([ + ('/index', 'file://'), + ('/index.html', 'file://'), + ('./index.html', 'file://'), + ('../index.html', 'file://'), + ('../../index.html', 'file://'), + ('./data/index.html', 'file://'), + ('.hidden/data/index.html', 'file://'), + ('/home/user/www/index.html', 'file://'), + ('//home/user/www/index.html', 'file://'), + ('file:///home/user/www/index.html', 'file://'), - def test_file_uri_relative004(self): - url = guess_scheme('subdir/index.html') - assert url.startswith('file://') + ('index.html', 'http://'), + ('example.com', 'http://'), + ('www.example.com', 'http://'), + ('www.example.com/index.html', 'http://'), + ('http://example.com', 'http://'), + ('http://example.com/index.html', 'http://'), + ('localhost', 'http://'), + ('localhost/index.html', 'http://'), - def test_file_uri_absolute001(self): - """Absolute file paths get prepended with "file://" scheme""" - iurl = '/home/user/www/index.html' - url = guess_scheme(iurl) - self.assertEquals(url, 'file://'+iurl) + # some corner cases (default to http://) + ('/', 'http://'), + ('.../test', 'http://'), - def test_file_uri_scheme(self): - """Output File URI does not change if "file://" scheme is set""" - iurl = 'file:///home/user/www/index.html' - url = guess_scheme(iurl) - self.assertEquals(url, iurl) + ], start=1): + t_method = create_guess_scheme_t(args) + t_method.__name__ = 'test_uri_%03d' % k + setattr (ShellURLTest, t_method.__name__, t_method) - def test_file_uri_windows(self): - raise unittest.SkipTest("Windows filepath are not supported for scrapy shell") - url = guess_scheme('C:\absolute\path\to\a\file.html') - assert url.startswith('file://') - - def test_http_url_001(self): - url = guess_scheme('index.html') - assert url.startswith('http://') - - def test_http_url_002(self): - url = guess_scheme('example.com') - assert url.startswith('http://') - - def test_http_url_003(self): - url = guess_scheme('www.example.com') - assert url.startswith('http://') - - def test_http_url_004(self): - url = guess_scheme('www.example.com/index') - assert url.startswith('http://') - - def test_http_url_005(self): - url = guess_scheme('www.example.com/index.html') - assert url.startswith('http://') - - def test_http_url_scheme(self): - """An full HTTP URL is unaltered""" - iurl = 'http://www.example.com/index.html' - url = guess_scheme(iurl) - self.assertEquals(url, iurl) +# TODO: the following tests do not pass with current implementation +for k, args in enumerate ([ + ('C:\absolute\path\to\a\file.html', 'file://', + 'Windows filepath are not supported for scrapy shell'), + ], start=1): + t_method = create_skipped_scheme_t(args) + t_method.__name__ = 'test_uri_skipped_%03d' % k + setattr (ShellURLTest, t_method.__name__, t_method) class ShellTest(ProcessTest, SiteTest, unittest.TestCase):