classification
Title: Remove human verification from test suite (test_parser and test_subprocess)
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: benjamin.peterson, eric.araujo, ezio.melotti, mark.dickinson, python-dev, terry.reedy
Priority: normal Keywords: patch

Created on 2011-04-30 15:43 by eric.araujo, last changed 2013-03-11 14:26 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
real-assert-in-test_parser.diff eric.araujo, 2011-04-30 15:43 review
issue11963.diff ezio.melotti, 2013-03-09 22:44 Patch against 2.7 to fix test_subprocess.
issue11963.diff ezio.melotti, 2013-03-09 23:20 Same patch but against 3.x. review
Messages (15)
msg134868 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-30 15:43
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?
msg134896 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-05-01 03:01
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.
msg135311 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-05-06 16:01
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?
msg135377 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-05-06 22:40
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'.
msg135465 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-05-07 14:19
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.
msg135485 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-05-07 17:20
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.)
msg136025 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-05-15 09:53
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.
msg183845 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-09 20:21
New changeset 9ee8c00f7e63 by Ezio Melotti in branch '2.7':
#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':
#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':
#11963: merge with 3.2.
http://hg.python.org/cpython/rev/185c923f21ec

New changeset acf6ffc57fcf by Ezio Melotti in branch 'default':
#11963: merge with 3.3.
http://hg.python.org/cpython/rev/acf6ffc57fcf
msg183852 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-09 22:44
> 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.
msg183858 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-10 01:30
New changeset 61ec83956ba6 by Ezio Melotti in branch '2.7':
#11963: fix Windows buildbots.
http://hg.python.org/cpython/rev/61ec83956ba6

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

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

New changeset 65147d2422dc by Ezio Melotti in branch 'default':
#11963: merge with 3.3.
http://hg.python.org/cpython/rev/65147d2422dc
msg183902 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-10 22:37
New changeset d25749c32bb4 by Ezio Melotti in branch '2.7':
#11963: remove human verification from test_subprocess.
http://hg.python.org/cpython/rev/d25749c32bb4
msg183906 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-11 01:21
New changeset 3e14aafeca04 by Ezio Melotti in branch '2.7':
#11963: fix Windows buildbots.
http://hg.python.org/cpython/rev/3e14aafeca04
msg183919 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-11 04:03
New changeset 222505a5aeac by Ezio Melotti in branch '3.2':
#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':
#11963: merge with 3.2.
http://hg.python.org/cpython/rev/cc08036b37a4

New changeset 629dba9f6746 by Ezio Melotti in branch 'default':
#11963: merge with 3.3.
http://hg.python.org/cpython/rev/629dba9f6746
msg183920 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-11 04:08
This should be fixed now.
msg183964 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-03-11 14:26
Thanks!
History
Date User Action Args
2013-03-11 14:26:37eric.araujosetmessages: + msg183964
2013-03-11 04:08:34ezio.melottisetstatus: open -> closed
resolution: fixed
messages: + msg183920

stage: patch review -> resolved
2013-03-11 04:03:21python-devsetmessages: + msg183919
2013-03-11 01:21:27python-devsetmessages: + msg183906
2013-03-10 22:37:14python-devsetmessages: + msg183902
2013-03-10 01:30:13python-devsetmessages: + msg183858
2013-03-09 23:20:07ezio.melottisetfiles: + issue11963.diff
assignee: ezio.melotti
2013-03-09 22:44:44ezio.melottisetfiles: + issue11963.diff

messages: + msg183852
versions: + Python 2.7, Python 3.2, Python 3.4
2013-03-09 20:21:52python-devsetnosy: + python-dev
messages: + msg183845
2011-05-15 09:53:36eric.araujosetmessages: + msg136025
2011-05-07 17:20:27terry.reedysetmessages: + msg135485
2011-05-07 14:19:31eric.araujosetmessages: + msg135465
title: Use real assert* for test_trigger_memory_error (test_parser) -> Remove human verification from test suite (test_parser and test_subprocess)
2011-05-06 22:40:57terry.reedysetnosy: + terry.reedy
messages: + msg135377
2011-05-06 16:01:12eric.araujosetmessages: + msg135311
2011-05-01 03:01:53ezio.melottisetnosy: + ezio.melotti
messages: + msg134896
2011-04-30 15:43:23eric.araujocreate