Remove human verification from test suite (test_parser and test_subprocess)

[merwok]

BPO	11963
Nosy	@terryjreedy, @mdickinson, @benjaminp, @ezio-melotti, @merwok
Files	real-assert-in-test_parser.diff issue11963.diff: Patch against 2.7 to fix test_subprocess. issue11963.diff: Same patch but against 3.x.

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ezio-melotti'
closed_at = <Date 2013-03-11.04:08:34.711>
created_at = <Date 2011-04-30.15:43:23.927>
labels = ['type-bug', 'tests']
title = 'Remove human verification from test suite (test_parser and test_subprocess)'
updated_at = <Date 2013-03-11.14:26:37.258>
user = 'https://github.com/merwok'

bugs.python.org fields:

activity = <Date 2013-03-11.14:26:37.258>
actor = 'eric.araujo'
assignee = 'ezio.melotti'
closed = True
closed_date = <Date 2013-03-11.04:08:34.711>
closer = 'ezio.melotti'
components = ['Tests']
creation = <Date 2011-04-30.15:43:23.927>
creator = 'eric.araujo'
dependencies = []
files = ['21842', '29359', '29360']
hgrepos = []
issue_num = 11963
keywords = ['patch']
message_count = 15.0
messages = ['134868', '134896', '135311', '135377', '135465', '135485', '136025', '183845', '183852', '183858', '183902', '183906', '183919', '183920', '183964']
nosy_count = 6.0
nosy_names = ['terry.reedy', 'mark.dickinson', 'benjamin.peterson', 'ezio.melotti', 'eric.araujo', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue11963'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4']

[merwok]

I’ve always found strange that one test relied on visual (i.e. human) checking instead of using an assert* method. I changed it and found that the test still passed (see attached patch). Is there any reason to do it the old way?

[ezio-melotti]

I think the patch is OK.
FWIW I tried to replace sys.stderr with a StringIO to see if it was possible to keep using assertRaises and then read the value from the StringIO but that didn't work.

[merwok]

IIRC the code uses C stdio to print its error message, not Python APIs, so this would explain why replacing sys.stdout doesn’t work.

There’s another test that spits things to stdout: test_subprocess. Should I clean it up too?

[terryjreedy]

Unlike test.support, whose doc was recently expanded by a patch by Eli Bendersky, I see no mention of test.script helper in the test doc. [Do you think there should be?]. I was not aware of it. It might be newer than the 'old way'.

I would have titled this 'Removed human verification from test suite'.

[merwok]

Please feel free to improve titles; we’re a team, there’s no ego in bug title phrasing :) I did so on at least one bug of yours; hope you didn’t consider it rude.

Are you asking about the docstring of the test module or the reST doc of test.support? A brief mention of test.script_helper (and possibly the other undocumented test helpers) could be added in either or both IMO, but just to let people know about about the file and go read it or pydoc it; we don’t want to spend too much time documenting private modules with no compat constraints.

[terryjreedy]

My title suggestion was meant to say "Yes, if you are willing to expand the scope of this issue and do more work, go ahead' ;-).

I have not looked at help(test), but it should be complete if it is not.
I was referring to the test module doc 25.5. Someone decided recently that test.support 25.6 should be complete, for the benefit of developers (like me) even though subject to change. Hence the recent patch. That chapter should probably start with a notice that these are private interfaces, subject to change, for testing of Python.

I'd like to see at least a listing and brief description of other stuff I should know about, perhaps in a new 25.6.2 section. An alternative would be something in the developer docs. Looking at the listing of /test is a bit confusing. I have pretty much ignored everything other than test_x (and regrtest.py), assuming that they were data files used by particular tests. Hence I was surprised by script_helper.py as an annex to support.py. Are there others (that are not single-test specific)? (I know, this should become another issue.)

[merwok]

I had a shot at test_subprocess but it’s harder than test_parser. The method that outputs to stdout tests the behavior of Popen(..., stdout=None). Putting another layer of indirection with script_helper could obfuscate it to the point of making it unreadable (what stdout are we talking about? It’s a subprocess in a subprocess in a test). I’ll try to do it in a few weeks but will not commit it without review.

[python-dev]

New changeset 9ee8c00f7e63 by Ezio Melotti in branch '2.7':
bpo-11963: avoid printing messages in test_parser. Initial patch by Éric Araujo.
http://hg.python.org/cpython/rev/9ee8c00f7e63

New changeset 10a82140f36d by Ezio Melotti in branch '3.2':
bpo-11963: avoid printing messages in test_parser. Initial patch by Éric Araujo.
http://hg.python.org/cpython/rev/10a82140f36d

New changeset 185c923f21ec by Ezio Melotti in branch '3.3':
bpo-11963: merge with 3.2.
http://hg.python.org/cpython/rev/185c923f21ec

New changeset acf6ffc57fcf by Ezio Melotti in branch 'default':
bpo-11963: merge with 3.3.
http://hg.python.org/cpython/rev/acf6ffc57fcf

[ezio-melotti]

Putting another layer of indirection with script_helper could obfuscate
it to the point of making it unreadable (what stdout are we talking
about? It’s a subprocess in a subprocess in a test).

The attached patch does this. The basic idea for test_stdout_none is that the subprocess inherits the stdout of the parent if stdout=None.
If the test launches a subprocess with stdout=None, the subprocess will print on the test's stdout. This is what currently happens, and that's why you see "this bit of output is from a test of stdout in a different process" while running test_subprocess. If we add another layer, we have the test that launches a subprocess (parent), that in turn launches another subprocess (child). The child process inherits the stdout of the parent process, and prints on it, and the test can then check the value of the parent stdout.

For test_stdout_filedes_of_stdout I used the same method, and everything seems to work fine.

[python-dev]

New changeset 61ec83956ba6 by Ezio Melotti in branch '2.7':
bpo-11963: fix Windows buildbots.
http://hg.python.org/cpython/rev/61ec83956ba6

New changeset 64b87578c071 by Ezio Melotti in branch '3.2':
bpo-11963: fix Windows buildbots.
http://hg.python.org/cpython/rev/64b87578c071

New changeset f683ca2b30e3 by Ezio Melotti in branch '3.3':
bpo-11963: merge with 3.2.
http://hg.python.org/cpython/rev/f683ca2b30e3

New changeset 65147d2422dc by Ezio Melotti in branch 'default':
bpo-11963: merge with 3.3.
http://hg.python.org/cpython/rev/65147d2422dc

[python-dev]

New changeset d25749c32bb4 by Ezio Melotti in branch '2.7':
bpo-11963: remove human verification from test_subprocess.
http://hg.python.org/cpython/rev/d25749c32bb4

[python-dev]

New changeset 3e14aafeca04 by Ezio Melotti in branch '2.7':
bpo-11963: fix Windows buildbots.
http://hg.python.org/cpython/rev/3e14aafeca04

[python-dev]

New changeset 222505a5aeac by Ezio Melotti in branch '3.2':
bpo-11963: remove human verification from test_parser and test_subprocess.
http://hg.python.org/cpython/rev/222505a5aeac

New changeset cc08036b37a4 by Ezio Melotti in branch '3.3':
bpo-11963: merge with 3.2.
http://hg.python.org/cpython/rev/cc08036b37a4

New changeset 629dba9f6746 by Ezio Melotti in branch 'default':
bpo-11963: merge with 3.3.
http://hg.python.org/cpython/rev/629dba9f6746

[ezio-melotti]

This should be fixed now.

[merwok]

Thanks!