classification
Title: Make test.script_helper more comprehensive, and use it in the test suite
Type: enhancement Stage: needs patch
Components: Tests Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Rodrigue.Alcazar, ezio.melotti, michael.foord, ncoghlan, pitrou, r.david.murray
Priority: normal Keywords: easy, patch

Created on 2010-08-04 22:50 by pitrou, last changed 2012-10-24 23:08 by ezio.melotti.

Files
File name Uploaded Description Edit
script_helper_del_refcount.patch r.david.murray, 2010-12-08 13:14 review
Messages (13)
msg112917 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-04 22:50
test.script_helper has a couple of dedicated functions to launch a Python interpreter instance in a subprocess. Unfortunately, it is little used and most test modules use their own ad hoc calls to subprocess instead.

Remedying the situation would require:
- adding functions to script_helper (currently, the available functions merge stdout and stderr together, which is clearly undesireable)
- perhaps improve the existing functions (kill_python() does a strange dance instead of calling communicate() on the subprocess.Popen object, is there a reason for that?)
- convert most uses of subprocess.<some_func>([sys.executable, ...]) in the test suite to use script_helper instead

This was suggested by Nick in issue477863.
msg112969 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-08-05 10:35
(The email daemon was not in a happy place, so posting directly)

On Thu, Aug 5, 2010 at 8:50 AM, Antoine Pitrou <report@bugs.python.org> wrote:
> - perhaps improve the existing functions (kill_python() does a strange dance instead of calling communicate() on the subprocess.Popen object, is there a reason for that?)

script_helper just factored out the old test_cmd_line approach which
was in turn based on a minimalistic change from a popen2 based
implementation (see
http://svn.python.org/view/python/trunk/Lib/test/test_cmd_line.py?r1=54386&r2=55245).
Doing something smarter probably isn't a bad idea.
msg113145 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-08-06 23:40
One other feature for the new-and-improved helpers: add a flag to allow "-E" to be omitted (as per the comment in test_cmd_line)
msg119517 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-10-24 14:05
I still think this is a good idea, I'm just not actively working on it. It might make a good project for someone wanting to get to know the process of working on CPython without having to deal with anything that is particularly tricky to understand.
msg119741 - (view) Author: Rodrigue Alcazar (Rodrigue.Alcazar) Date: 2010-10-27 21:37
> someone wanting to get to know the process of working on CPython without having to deal with anything that is particularly tricky to understand.

That sounds exactly like me :)

I can have a look at this ticket.
msg123503 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-06 20:53
I just tried using script_helper in a new test, so I have a couple of comments.

I don't see stdout and stderr being conflated, it looks to me like they are returned separately, at least by the assert methods.

The assert methods return results, which is unlike other assert methods.  This is very useful, even essential, and I wouldn't want to give it up.  That conflicts with the current unittest conventions, though.

It would be a big help if 'err' were returned with the refcount line removed if it is there, which would make tests using the methods return the same 'err' regardless of whether they are run under a debug build or not.

I think the names of the two assert functions should follow the current unit test conventions (assertPythonRunOK and asssertPythonRunNotOK, perhaps?)
msg123504 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-06 21:02
> I just tried using script_helper in a new test, so I have a couple of
> comments.
> 
> I don't see stdout and stderr being conflated, it looks to me like
> they are returned separately, at least by the assert methods.

That's because I wrote the assert methods since this issue was opened :)

> It would be a big help if 'err' were returned with the refcount line
> removed if it is there, which would make tests using the methods
> return the same 'err' regardless of whether they are run under a debug
> build or not.

Indeed.

> I think the names of the two assert functions should follow the
> current unit test conventions (assertPythonRunOK and
> asssertPythonRunNotOK, perhaps?)

Well, they are functions, not methods, so I don't think they have to
follow the other convention.
msg123516 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-07 02:51
OK, fine on the convention, but I'd still like a more memorable name for assert_python_failure.  I've been working on this issue off and on today, and I've had to look up that name at least four times.  I can remember assert_python_ok, but I can't remember whether its inverse is assert_python_fails, assert_python_bad, or what.  For some reason I haven't guessed 'failure' even once so far :)  (I know it's not assert_python_not_ok because I remember it isn't parallel...)
msg123601 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-08 13:14
Here is a patch that causes _assert_python to remove the refcount lines from stderr.
msg123602 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-08 13:22
Hmm.  Having posted that it occurs to me that it could be useful to have the _remove_refcount function in test.support as remove_refcount instead.
msg123633 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-08 18:54
> Having posted that it occurs to me that it could be useful to have the 
> _remove_refcount function in test.support

There's already strip_python_stderr() :)
msg123637 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-08 19:29
Oh, good, I'll use that then.  I could have sworn I looked for that functionality a couple weeks ago and couldn't find it.
msg173718 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-10-24 23:08
> There's already strip_python_stderr() :)

This is already used in Lib/test/script_helper.py:42, and a way to call _assert_python() without using -E has also been introduced[0].

The next step would be to check if these functions could be used elsewhere in the other tests.  Also note that these functions are not currently documented anywhere.

[0]: http://hg.python.org/cpython/diff/f4b7ecf8a5f8/Lib/test/script_helper.py
History
Date User Action Args
2012-10-24 23:08:07ezio.melottisetstage: needs patch
messages: + msg173718
versions: + Python 3.4, - Python 3.2
2010-12-08 19:29:29r.david.murraysetmessages: + msg123637
2010-12-08 18:54:59pitrousetmessages: + msg123633
2010-12-08 13:22:55r.david.murraysetmessages: + msg123602
2010-12-08 13:14:52r.david.murraysetfiles: + script_helper_del_refcount.patch
keywords: + patch
messages: + msg123601
2010-12-07 02:51:30r.david.murraysetmessages: + msg123516
2010-12-06 21:02:58pitrousetmessages: + msg123504
2010-12-06 20:59:14r.david.murraysetnosy: + michael.foord
2010-12-06 20:53:16r.david.murraysetnosy: + r.david.murray
messages: + msg123503
2010-10-27 21:37:25Rodrigue.Alcazarsetnosy: + Rodrigue.Alcazar
messages: + msg119741
2010-10-24 14:05:04ncoghlansetkeywords: + easy
assignee: ncoghlan ->
messages: + msg119517
2010-08-06 23:40:50ncoghlansetassignee: ncoghlan
messages: + msg113145
2010-08-05 10:35:16ncoghlansetmessages: + msg112969
2010-08-04 22:50:46pitroucreate