classification
Title: test_cmd_line_script should include more sys.argv checks
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Jason.Yeo, eric.araujo, ncoghlan, python-dev, tshepang
Priority: normal Keywords: easy, patch

Created on 2012-02-15 23:26 by ncoghlan, last changed 2012-04-22 07:20 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
14026v2.patch Jason.Yeo, 2012-02-21 00:56 14026v2patch review
Messages (11)
msg153448 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-02-15 23:26
Currently, test_cmd_line_script only checks sys.argv[0] is set correctly.

It should pass some extra values after the script name, then include an appropriate check in the output of the launched script.
msg153526 - (view) Author: Jason Yeo (Jason.Yeo) * Date: 2012-02-17 02:05
I would like to work on this but I am not sure how to go about it. It seems that the method signature for _check_script has to be changed in include another parameter for expected_argv1, expected_argv2, etc. The _check_output also has to be changed to include assertIn(printed_argv1, data) for the additional parameters. This also means that test_source has to include some lines to print the sys.argv[1], sys.argv[2], etc. 

Am I on the right track? Or is there a easier way?

(By the way, when I set verbose to a value > 1, errors are found. I realized that a print() statement was printing an undefined variable. I have filed an issue and attached a patch to it. The link for the issue is http://bugs.python.org/issue14032)
msg153562 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-17 16:22
> It seems that the method signature for _check_script has to be changed
> in include another parameter for expected_argv1, expected_argv2,
It would seem simpler to me to make that expected_argv :)
msg153602 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-02-17 21:49
For the purposes of this test (i.e. running the same script several different ways and making sure it always behaves as expected), I wouldn't even worry about making it configurable.

Just define a list of example args as a module global, append them to the run_args in _check_script, and check them against sys.argv[1:] with assertEqual() in the test script.
msg153647 - (view) Author: Jason Yeo (Jason.Yeo) * Date: 2012-02-18 16:05
>Just define a list of example args as a module global, append them to 
>the run_args in _check_script, and check them against sys.argv[1:] with 
>assertEqual() in the test script.

Okay I have done that but the assertion passes in all the tests except in test_issue8202 and test_issue8202_dash_m_file_ignored. It seems that the script didn't receive any arguments from the cmd line from the way it is executed in these functions. 

I have attached a patch to let you see what I am doing. Let me know if I am wrong. Thanks.
msg153654 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-18 19:51
I don’t understand why you import the module inside the same module.
msg153661 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-02-19 00:16
The second instance is inside the source code string that's written out as the script to be run in the subprocess. Not a bad idea actually:
- it avoids writing the example args twice (which is what I was thinking of doing)
- it avoids turning the test_script into a string formatting template (which is something I've been trying to avoid for that test)
- it provides an additional sanity check on how sys.path is being initialised in the subprocess

For the two failures, I suggest modifying _check_script() to return the "rc, out, err" from the underlying assert_python_ok() call, then updating the two offending tests to call _check_script() appropriately instead of calling assert_python_ok() directly.
msg153826 - (view) Author: Jason Yeo (Jason.Yeo) * Date: 2012-02-21 00:55
>For the two failures, I suggest modifying _check_script() to return the 
>"rc, out, err" from the underlying assert_python_ok() call

I've decided to simple pass in *example_args into the assert_python_ok() in those two offending tests. There are less changes this way. Please let me know if I am doing this correctly. Thanks. ;)
msg154163 - (view) Author: Jason Yeo (Jason.Yeo) * Date: 2012-02-24 21:08
*friendly ping*, how's the review for this patch?
msg154188 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 05:54
I saw nothing wrong.  Give Nick a week to find time to review :)
msg158950 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-22 07:17
New changeset 22f0044ea366 by Nick Coghlan in branch '3.2':
Close issue #14026 by better testing sys.argv handling in test_cmd_line_script (patch by Jason Yeo)
http://hg.python.org/cpython/rev/22f0044ea366

New changeset ff6593aa8376 by Nick Coghlan in branch 'default':
Resolve #14026 (Merge from 3.2)
http://hg.python.org/cpython/rev/ff6593aa8376
History
Date User Action Args
2012-04-22 07:20:53ncoghlansetstatus: open -> closed
stage: test needed -> resolved
resolution: fixed
versions: - Python 2.7
2012-04-22 07:17:37python-devsetnosy: + python-dev
messages: + msg158950
2012-02-27 14:08:58tshepangsetnosy: + tshepang
2012-02-25 06:34:51ncoghlansetassignee: ncoghlan
2012-02-25 05:54:08eric.araujosetmessages: + msg154188
2012-02-24 21:08:45Jason.Yeosetmessages: + msg154163
2012-02-21 00:58:08Jason.Yeosetfiles: - 14026patch
2012-02-21 00:56:37Jason.Yeosetfiles: + 14026v2.patch
keywords: + patch
2012-02-21 00:55:34Jason.Yeosetfiles: - mypatch
2012-02-21 00:55:01Jason.Yeosetfiles: + mypatch

messages: + msg153826
2012-02-19 00:16:05ncoghlansetmessages: + msg153661
2012-02-18 19:51:38eric.araujosetmessages: + msg153654
2012-02-18 16:05:37Jason.Yeosetfiles: + 14026patch

messages: + msg153647
2012-02-17 21:49:50ncoghlansetmessages: + msg153602
2012-02-17 16:30:18eric.araujosetmessages: - msg153560
2012-02-17 16:22:11eric.araujosetmessages: + msg153562
2012-02-17 16:19:55eric.araujosetmessages: + msg153560
2012-02-17 02:05:10Jason.Yeosetmessages: + msg153526
2012-02-16 14:21:26Jason.Yeosetnosy: + Jason.Yeo
2012-02-16 02:33:54eric.araujosetnosy: + eric.araujo
stage: test needed
type: enhancement

versions: + Python 2.7, Python 3.2
2012-02-15 23:26:11ncoghlancreate