classification
Title: Fix test discovery for test_robotparser.py
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: brett.cannon, ezio.melotti, python-dev, zach.ware
Priority: normal Keywords: patch

Created on 2013-01-28 20:32 by zach.ware, last changed 2013-03-12 05:54 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
test_robotparser_discovery.diff zach.ware, 2013-01-28 20:32 test_robotparser.py fix, version 1 review
test_robotparser_discovery.v2.diff zach.ware, 2013-03-04 21:34 Version 2 review
test_robotparser_discovery.v3.diff zach.ware, 2013-03-04 21:35 Version 3 (approach 2, version 1) review
Messages (6)
msg180878 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-01-28 20:32
Here's a fix for test_robotparser.py.  With this patch, the command 'python -m unittest discover Lib/test/ "test_*.py"' can actually be run--before the patch, test_robotparser's unique TestCase subclass causes unexpected errors for discovery.
msg183499 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-03-04 21:34
I got to looking over my patch here again, and thought of another possible fix.  I'm not sure which is uglier, though :)

The problem is that most of the test cases are created using a custom subclass of unittest.TestCase which takes extra constructor arguments.  Discovery doesn't expect a TestCase subclass to want extra arguments, so it fails noisily trying to 'discover' RobotTestCase.

Patch version 2 just makes a couple small adjustments to version 1, which replaces the custom subclass with a base class to provide runTest, and a factory function to create the TestCase instances.

Version 3 is a different approach, which provides default arguments to the extra constructor parameters, and bails out of __init__ early if the index argument is not an int (checking for None doesn't work because at some point, discovery calls 'RobotTestCase("runTest")', which makes 'index' "runTest").

Both approaches look kind of ugly to me, but both get the job done.
msg183895 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-10 20:24
Do you think doing something like:

class BaseRobotTest:
    def setUp(self):
        lines = io.StringIO(robots_txt).readlines()
        self.parser = urllib.robotparser.RobotFileParser()
        parser.parse(lines)

    def test_good(self):
         for url in good:
             self.assertTrue(self.parser.can_fetch(...))

    def test_bad(self):
         for url in bad:
             self.assertFalse(self.parser.can_fetch(...))

class RobotTestX(BaseRobotTest, unittest.TestCase):
    doc = "..."
    good = [...]
    bad = [...]

...

would be a better approach?

On one hand is a bit more verbose and doesn't create a separate test for each URL (I don't think that's important though), but on the other hand it gets rid of lot of magic and makes the test more understandable.
msg183972 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-03-11 16:41
I agree that, long-term, test_robotparser should probably be rewritten with less magic.  Short-term, though, I think it would be good to get a temporary fix for discovery in place since this is the only test file that completely blows up discovery before it even starts testing, and open a new issue for the rewrite.  The v3 patch does the job with the least disruption in the file, though it does add a bit more magic.
msg184009 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-12 05:51
New changeset fa051c6276a0 by Ezio Melotti in branch '3.3':
#17066: test_robotparser now works with unittest test discovery.  Patch by Zachary Ware.
http://hg.python.org/cpython/rev/fa051c6276a0

New changeset 8fdce849d0b3 by Ezio Melotti in branch 'default':
#17066: merge with 3.3.
http://hg.python.org/cpython/rev/8fdce849d0b3
msg184010 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-12 05:54
Fixed, thanks for the patch!
History
Date User Action Args
2013-03-12 05:54:10ezio.melottisetstatus: open -> closed
messages: + msg184010

assignee: ezio.melotti
resolution: fixed
stage: resolved
2013-03-12 05:51:04python-devsetnosy: + python-dev
messages: + msg184009
2013-03-11 16:41:32zach.waresetmessages: + msg183972
2013-03-10 20:24:20ezio.melottisetmessages: + msg183895
2013-03-04 21:35:18zach.waresetfiles: + test_robotparser_discovery.v3.diff
2013-03-04 21:34:47zach.waresetfiles: + test_robotparser_discovery.v2.diff

messages: + msg183499
2013-01-28 20:32:51zach.warecreate