| msg125597 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2011-01-06 22:34 |
| r87812 shows that using getopt is not a good thing; having the short and long versions of an argument separated from each other can lead to bugs. It would be good to move test.regrtest over to argparse to help prevent that from happening again. |
|
|
| msg125599 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-06 22:44 |
| I had that in mind since quite some time, so I'm taking ownership of this issue. |
|
|
| msg125610 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-07 01:01 |
| Note that based on my experience with the conversion of compileall to argparse,it is important to have good tests. Of course, regrtest itself has no tests... |
|
|
| msg125965 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-11 00:17 |
| > R. David Murray <rdmurray@bitdance.com> added the comment: > > Note that based on my experience with the conversion of compileall to argparse,it is important to have good tests. Of course, regrtest itself has no tests... Indeed that's quite funny :) anyhow, any suggestions on how to properly test it while porting it to argparse? f.e, I can think to get all the examples in the devguide and make sure they work, but since I'm new to python development I might miss something really important, hence the commends from more experienced devs would be really important for me :) Cheers, Sandro |
|
|
| msg125975 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-11 01:24 |
| Testing regrtest is distinctly non-trivial, since options have interactions (some of the somewhat unobvious). Ideally we'd refactor the code so that we could point it at a test test-directory so we could write some automated tests for it :) But if you are going that far you might as well rewrite it. This is, I suspect, why nobody has yet done this conversion. My best suggestion if you really want to go ahead is to go through each option individually, with and without command line arguments, and test how it currently behaves and make good notes. Anything that doesn't make sense, ask on (#)python-dev. And then...build a matrix and test each option in combination with each other option, again keeping notes. Which is something that has probably never been done, and will doubtless reveal some interesting bugs. You might be able to automate some tests using doctest and subprocess. I'm not sure about that easy tag. This could easily be more than a one day project. I suspect you will find your fingers itching to refactor more than just the argument parsing code. You should probably resist that urge insofar as possible :) |
|
|
| msg125978 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-11 01:25 |
| Note that it is also possible that after doing a review of the functionality, there might be consensus to drop one or more options, which would be a good thing overall, IMO. |
|
|
| msg126948 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-24 19:50 |
| I finally had the time to look more closely to the issue, and I'd like to hear some comments on the info visualization. Currently we have --help option to print: <usage + additional details about execution + more rigorous testing> Now, AFAIK argparse it's not so flexible, so we have to draw a line and choose a trade-off. F.e., the additional options details: they can't be added in a help='..' kargs but they should be nonetheless be displayed when passing --help to regrtest.py. * , , , can all be grouped up into argument groups. * can be managed "twisting a bit" the usage='...' karg of argparse.ArgumentParser * but what about ? should we add that to the usage='...' text? or should we somehow override the print_help() and show <options & options groups> (this is standard until here) (appending the text after options)? or something different I still don't imagine? :) Cheers, Sandro |
|
|
| msg126949 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-24 19:53 |
| What about putting the addition option details in the epilog? |
|
|
| msg126965 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-24 22:32 |
| > R. David Murray <rdmurray@bitdance.com> added the comment: > > What about putting the addition option details in the epilog? maybe it loose the fact that all the doc/explanation for regrtest options were available from the command-line? what about a --more-help option that print_help() + all those information about cmdline switches? |
|
|
| msg126978 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-25 01:20 |
| That might be handy. I thought you were trying to roughly reproduce the current help (which dumps it all out at once), which is why I suggested epilog. |
|
|
| msg127021 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-25 17:11 |
| On Tue, Jan 25, 2011 at 02:20, R. David Murray <report@bugs.python.org> wrote: > That might be handy. I thought you were trying to roughly reproduce the current help (which dumps it all out at once), which is why I suggested epilog. actually that was my objective :) but facing the impossibility to implement it, I asked for advice; I'll go with --help with only usage & "classic" options descriptions and --more-help for --help + long options descriptions |
|
|
| msg127034 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-25 18:29 |
| Hmm. Am I misunderstanding something about epilog, then? I thought it was placed at the end of the other help text? |
|
|
| msg127038 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-25 18:39 |
| On Tue, Jan 25, 2011 at 19:29, R. David Murray <report@bugs.python.org> wrote: > > R. David Murray <rdmurray@bitdance.com> added the comment: > > Hmm. Am I misunderstanding something about epilog, then? I thought it was placed at the end of the other help text? Sorry I get confused by the assonance with epydoc (ok they are quite fare away but still :) now I see argparse.ArgumentParser has an 'epidoc' karg that does exactly what I meant: thanks! |
|
|
| msg127041 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2011-01-25 18:54 |
| ISTM that moving from argument parser to another is more likely to introduce bugs than to solve them. There may be other advantages, but reducing bugginess isn't one of them. Lots of scripts have used getopts successfully. |
|
|
| msg127043 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2011-01-25 19:10 |
| If Sandro is willing to write test for regrtest as part of the move then that would be a complete net win from the current situation. |
|
|
| msg127049 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2011-01-25 20:37 |
| +1 |
|
|
| msg127053 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-25 21:17 |
| Sure, that would be really interesting to do, and I do commit to write a "test suite to the tool that runs the python test suite" :) What I'm asking is: how would you do that? I'm quite new as contributor so the ideas of experienced core devs are very valuable at this stage of the task. David proposed to write a parallel test-directory for regrtest, would you think it's feasible to do that? and what to put in that dir to "trigger" weird behaviour in regrtest? Any other input? |
|
|
| msg127054 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-01-25 21:21 |
| Some parts of regrtest have been obsoleted by changes in unittest. Best not to write tests for something that will go. |
|
|
| msg127064 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-25 22:54 |
| I would say writing tests for regrtest is going to be a somewhat tricky task. I think you will have to do some code tweaking to even be able to run certain tests. I believe that regrtest currently has some built in assumptions about the test directory (ie: that it is Lib/test), even though the regrtest main program provides a testdir argument. So the first task is probably going to be getting that argument to actually work, and then exposing a way to set it at the regrtest command level (or, alternatively, moving all command functionality out of __main__ and into main). Once you have that, your test suite can programatically generate a test test-directory and populate it with whatever files you need for each test, using the utilities in test.script_helper. |
|
|
| msg127153 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-26 23:06 |
| As suggested by David, I made it possible to specify an alternative test directory by introducing '--testdir DIR' cli option: attached the patch, comments are welcome :) What about STDTESTS/NOTTESTS in case --testdir is specified? Currently they are executed/excluded even in case we're not running the python test suite: should we remove them in that case? |
|
|
| msg127154 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-26 23:13 |
| shouldn't we use the same method also for --coverdir (that currently faild the "least surprise" test when specifying a relative path) replacing coverdir = os.path.join(os.getcwd(), a) with coverdir = os.path.join(support.SAVEDCWD, a) ? |
|
|
| msg127157 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-26 23:31 |
| I would open three new bugs to address the issues you raise. It ought to be possible to rename things so that we can eliminate the pre-population of NOTTESTS (if not I'd like to know why not!). STDTESTS appear to move certain tests to the front, possibly for stability reasons? Perhaps that can be eliminated as well; certainly it seems worth investigating. If we can't eliminate it then a check could be added such that the tests would not be returned if they don't actually show up in the directory passed to findtests. The coverdir thing looks like a bug. |
|
|
| msg127172 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2011-01-27 03:54 |
| As I recall, STDTESTS is there to check we have a basically functioning interpreter (i.e. the compiler works, etc). The idea is that if any of those fail, everything else is likely to go belly up as well. If regrtest is being used to run some other set of tests, then that won't apply. I thought we actually did use the testdir arg to handle the PID specific directory naming on the buildbots, but I may be misrembering where that appears in the execution sequence. |
|
|
| msg127229 - (view) |
Author: Sandro Tosi (sandro.tosi) * |
Date: 2011-01-27 22:02 |
| I've created two new issues (David, I think I've lost why I'd need 3 :) ) * - finally allows to specify a relative dir with --coverdir * - to expose --testdir in order to specify a different location of the directory containing tests (disabling stdtest & nottest if testdir is set). Thanks Nick for your explanation of what's the purpose of STDTEST & NOTTESTS, I've added a brief comment in the code to write that in stone :) do you think should be in dependencies? |
|
|
| msg127232 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-01-27 22:29 |
| I would say so, otherwise how are you going to run the tests you write :) As for the other issue...I hadn't counted one for --testdir, but making that a new issue was a good idea. So the other two I had in mind was for STDTESTS and NOTTESTS. I still think the NOTTESTS pre-population should be eliminated by renaming the two problematic files, unless someone can explain why they need to be named test_xxx. For STDTESTS...you can fix that as part of 11031 (don't return them if they don't exist in the test dir). |
|
|
| msg168033 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-08-12 12:21 |
| I just discovered that issue 15302, which has a patch awaiting review from a month ago as well as some discussion, is a duplicate of this issue. Would it be possible to leave that issue open (retitling either or both issues if necessary to avoid overlap)? The discussion here seems broader in certain ways (e.g. discussing the behavior of certain options, pre-requisites for end-to-end testing of regrtest, adding a --more-help option, etc). The patch in issue 15302 is narrower in that it handles testing via dependency injection so that main() can remain largely unaffected. I could propose such a retitling. > What about putting the addition option details in the epilog? By the way, David, I did use epilog in my patch there (without having seen this thread). :) |
|
|
| msg178358 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-12-28 04:10 |
| Since regrtest is now using argparse (as of 6e2e5adc0400), is there a reason to keep this issue open? Or should the issue be retitled (current title: "Move test.regrtest from getopt to argparse")? There seem to be some thoughts in the comments that are broader than the current title, but I don't know the right focus. It seems to be more of a brainstorm on how to improve regrtest. |
|
|
| msg178421 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2012-12-28 20:04 |
| Since http://bugs.python.org/issue10967 is the meta issue for updating regrtest this can be closed. |
|
|