warning when ambiguous root arguments and minor cleanups

scrapy/linkextractors/sgml.py

@@ -127,11 +127,10 @@ class SgmlLinkExtractor(FilteringLinkExtractor):
     def extract_links(self, response):
         base_url = None
         if self.restrict_xpaths:
-            sel = response.selector
             base_url = get_base_url(response)
             body = u''.join(f
                             for x in self.restrict_xpaths
-                            for f in sel.xpath(x).extract()
+                            for f in response.xpath(x).extract()
                             ).encode(response.encoding, errors='xmlcharrefreplace')
         else:
             body = response.body

scrapy/selector/unified.py

@@ -33,10 +33,13 @@ class Selector(ParselSelector, object_ref):
     def __init__(self, response=None, text=None, type=None, root=None, _root=None, **kwargs):
         st = _st(response, type or self._default_type)
 
-        if root is None and _root is not None:
+        if _root is not None:
             warnings.warn("Argument `_root` is deprecated, use `root` instead",
                           ScrapyDeprecationWarning, stacklevel=2)
-            root = _root
+            if root is None:
+                root = _root
+            else:
+                warnings.warn("Ignoring deprecated `_root` argument, using provided `root`")
 
         if text is not None:
             response = _response_from_text(text, st)

tests/test_selector.py

@@ -8,7 +8,6 @@ from scrapy.http import TextResponse, HtmlResponse, XmlResponse
 from scrapy.selector import Selector
 from scrapy.selector.lxmlsel import XmlXPathSelector, HtmlXPathSelector, XPathSelector
 from lxml import etree
-from tests import mock
 
 
 class SelectorTestCase(unittest.TestCase):
@@ -39,14 +38,21 @@ class SelectorTestCase(unittest.TestCase):
         self.assertEqual([x.extract() for x in sel.xpath("concat(//input[@name='a']/@value, //input[@name='b']/@value)")],
                          [u'12'])
 
-    @mock.patch('scrapy.selector.unified.warnings')
-    def test_deprecated_root_argument(self, warnings):
-        root = etree.fromstring(u'<html/>')
-        sel = self.sscls(_root=root)
-        self.assertIs(root, sel.root)
-        warnings.warn.assert_called_once_with(
-            'Argument `_root` is deprecated, use `root` instead',
-            mock.ANY, stacklevel=2)
+    def test_deprecated_root_argument(self):
+        with warnings.catch_warnings(record=True) as w:
+            root = etree.fromstring(u'<html/>')
+            sel = self.sscls(_root=root)
+            self.assertIs(root, sel.root)
+            self.assertEqual(str(w[-1].message),
+                             'Argument `_root` is deprecated, use `root` instead')
+
+    def test_deprecated_root_argument_ambiguous(self):
+        with warnings.catch_warnings(record=True) as w:
+            _root = etree.fromstring(u'<xml/>')
+            root = etree.fromstring(u'<html/>')
+            sel = self.sscls(_root=_root, root=root)
+            self.assertIs(root, sel.root)
+            self.assertIn('Ignoring deprecated `_root` argument', str(w[-1].message))
 
     def test_representation_slice(self):
         body = u"<p><input name='{}' value='\xa9'/></p>".format(50 * 'b')