classification
Title: start using argparse.Namespace in regrtest
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: asvetlov, chris.jerdonek, eli.bendersky, kushal.das, python-dev, serhiy.storchaka, tshepang
Priority: normal Keywords: patch

Created on 2012-12-28 03:11 by chris.jerdonek, last changed 2013-08-29 09:34 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue-16799-1.diff chris.jerdonek, 2012-12-29 01:45 review
regrtest_argparse.patch serhiy.storchaka, 2013-08-12 19:41 review
regrtest_argparse_2.patch serhiy.storchaka, 2013-08-28 10:33 review
Messages (16)
msg178356 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-28 03:11
Issue 15302 switched regrtest from getopt to argparse for parsing options.  However, regrtest.main() still expects and operates on getopt-style options.

This issue is to continue the regrtest refactoring and replace the use of getopt-style options with an argparse Namespace object.

This issue should probably be a meta-issue with the transition happening over several issues/commits, as there are many command-line options that will probably have varying types and actions (in the sense of argparse).  Options can be switched over incrementally in groups (e.g. by having the _parse_args() function return the parsed options in both forms: in both getopt-style format and a Namespace object).

This issue will be completed when the namespace-to-getopt "bridge code" is removed -- probably along with the corresponding argparse-to-getopt tests.
msg178461 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-12-29 01:45
Here is a patch to start using a Namespace object.

I also noticed that the usage() function I removed in 6e2e5adc0400 is still used elsewhere in regrtest.main(), so this patch also fixes that.
msg179014 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2013-01-04 10:40
The patches look good. Applied successfully and tests ran ok.
msg179065 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-04 18:29
Thanks.  Andrew, could you also take a quick look at this?
msg179215 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-06 20:01
May be it would be better to split the patch to two parts -- the fix of usage() bug and the enhancement.
msg179218 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-06 20:25
Sure, I'd be happy to do that.  I'll prepare the patch for the other issue.
msg179306 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-08 02:19
FYI, the fix for issue 16854 committed most of this patch, so the patch should be updated before reviewing.
msg194915 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-08-11 23:10
Chris, I was reading through regrtest.py for other reasons and stumbled upon the pointer to this issue. Would you like to update your patch? I will review it.
msg194934 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-12 09:35
See also issue17974.
msg194998 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-12 19:41
Here is a preliminary patch which get rids of _convert_namespace_to_getopt() and directly uses a Namespace object. Unfortunately it breaks tests because test_regrtest depends on implementation details of the regrtest module and uses _convert_namespace_to_getopt(). If the proposed patch is good enough I will try to write suitable tests.
msg195009 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-08-12 20:33
On Mon, Aug 12, 2013 at 12:41 PM, Serhiy Storchaka
<report@bugs.python.org>wrote:

>
> Serhiy Storchaka added the comment:
>
> Here is a preliminary patch which get rids of
> _convert_namespace_to_getopt() and directly uses a Namespace object.
> Unfortunately it breaks tests because test_regrtest depends on
> implementation details of the regrtest module and uses
> _convert_namespace_to_getopt(). If the proposed patch is good enough I will
> try to write suitable tests.
>

If test_regrtest is the only test broken by this, it doesn't like a major
problem.
msg195010 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-08-12 20:37
On Mon, Aug 12, 2013 at 1:33 PM, Eli Bendersky <report@bugs.python.org>wrote:

>
> Eli Bendersky added the comment:
>
> On Mon, Aug 12, 2013 at 12:41 PM, Serhiy Storchaka
> <report@bugs.python.org>wrote:
>
> >
> > Serhiy Storchaka added the comment:
> >
> > Here is a preliminary patch which get rids of
> > _convert_namespace_to_getopt() and directly uses a Namespace object.
> > Unfortunately it breaks tests because test_regrtest depends on
> > implementation details of the regrtest module and uses
> > _convert_namespace_to_getopt(). If the proposed patch is good enough I
> will
> > try to write suitable tests.
> >
>
> If test_regrtest is the only test broken by this, it doesn't like a major
> problem.
>

Something is wrong with the part of my brain that deals with grammar today;
sorry! What I mean to say, in case it's not obvious - is that if this
change breaks no tests except the one that specifically tests regrtest
itself, I don't think there's a reason to worry. test_regrtest can (and
should) be changed not to rely on internals of the module-under-test.
msg195333 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-08-16 14:10
Looks pretty good, Serhiy. I left some comments in Rietveld.
msg195384 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-16 18:13
Thank you for the review Eli. Could you please review issue17974 while I will write tests?

As for a duplication I think about using vars():


def main(tests=None, testdir=None, verbose=0, quiet=False, ...):
    return _main(**vars())

def _main(tests, testdir, **kwargs):
    ns = argparse.Namespace(**kwargs)
    ...
msg196371 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-28 10:33
Here is updated patch with tests.

Please open separated issues if you want suggest semantic changes (removing, renaming or changing behavior of some options).
msg196450 - (view) Author: Roundup Robot (python-dev) Date: 2013-08-29 09:27
New changeset 997de0edc5bd by Serhiy Storchaka in branch 'default':
Issue #16799: Switched from getopt to argparse style in regrtest's argument
http://hg.python.org/cpython/rev/997de0edc5bd
History
Date User Action Args
2013-08-29 09:34:43serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: test needed -> resolved
2013-08-29 09:27:12python-devsetnosy: + python-dev
messages: + msg196450
2013-08-28 10:33:53serhiy.storchakasetfiles: + regrtest_argparse_2.patch

messages: + msg196371
2013-08-16 18:14:27serhiy.storchakasetassignee: serhiy.storchaka
2013-08-16 18:13:49serhiy.storchakasetstage: test needed
2013-08-16 18:13:34serhiy.storchakasetmessages: + msg195384
2013-08-16 14:10:11eli.benderskysetmessages: + msg195333
2013-08-12 20:37:45eli.benderskysetmessages: + msg195010
2013-08-12 20:33:34eli.benderskysetmessages: + msg195009
2013-08-12 19:41:18serhiy.storchakasetfiles: + regrtest_argparse.patch

messages: + msg194998
2013-08-12 09:35:06serhiy.storchakasetmessages: + msg194934
2013-08-11 23:10:58eli.benderskysetnosy: + eli.bendersky
messages: + msg194915
2013-01-08 02:19:09chris.jerdoneksetmessages: + msg179306
2013-01-06 20:52:41chris.jerdonekunlinkissue16854 dependencies
2013-01-06 20:25:13chris.jerdoneksetmessages: + msg179218
2013-01-06 20:01:54serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg179215
2013-01-04 18:29:16chris.jerdoneksetmessages: + msg179065
title: switch regrtest from getopt options to argparse Namespace -> start using argparse.Namespace in regrtest
2013-01-04 10:40:50kushal.dassetnosy: + kushal.das
messages: + msg179014
2013-01-03 19:54:08chris.jerdoneklinkissue16854 dependencies
2012-12-29 01:45:15chris.jerdoneksetfiles: + issue-16799-1.diff
keywords: + patch
messages: + msg178461
2012-12-28 18:42:24asvetlovsetnosy: + asvetlov
2012-12-28 18:14:19tshepangsetnosy: + tshepang
2012-12-28 03:18:30chris.jerdoneksettitle: switch regrtest from getopt-style options to argparse Namespace object -> switch regrtest from getopt options to argparse Namespace
2012-12-28 03:11:11chris.jerdonekcreate