[issue15302] Use argparse instead of getopt in test.regrtest
Roundup Robot added the comment: New changeset ce99559efa46 by Chris Jerdonek in branch 'default': Issue #16854: Fix regrtest.usage() regression introduced in 6e2e5adc0400. http://hg.python.org/cpython/rev/ce99559efa46 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: Good step! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Updating patch after Benjamin's review. In this new patch, in test_regrtest I now use the current, actual getopt code to test and demonstrate backwards compatibility. Note that when I pasted the code, I also fixed the three typos in the current getopt code that are documented in the following three issues: issue15324, issue15325, issue15326. These typos are checked by the following test cases: test_fromfile, test_match, and test_randomize. (I will fix these three issues in versions prior to 3.4 independent of this issue.) -- Added file: http://bugs.python.org/file28452/issue-15302-6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Rietveld is erroring out on me again whenever I try to reply to a comment, so I'm posting my comment here. On 2012/12/27 18:29:22, Benjamin Peterson wrote: On 2012/12/27 04:44:33, Benjamin Peterson wrote: if val: Again, we need this to match getopt behavior. Using if val would replace the value provided for all options accepting a value with the empty string. In particular, the test_fromfile() test case (and two others) would fail with that change. On the flip side, getopt causes provided boolean options to show up as having an empty string value in the return value which is why I'm setting val to ''. I suppose this stuff is temporary anyway until regrtest can actualy use a nice namespace. Right. That part is a temporary bridge. I updated the patch based on your original comments, by the way. The patch is a lot simpler now, including dropping the need to subclass ArgumentParser, taking out the old usage() function, simplifying the tests, and making the need for the tests more explicit (which also can be removed after switching to using a namespace object). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Roundup Robot added the comment: New changeset 6e2e5adc0400 by Chris Jerdonek in branch 'default': Issue #15302: Switch regrtest from using getopt to using argparse. http://hg.python.org/cpython/rev/6e2e5adc0400 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Thanks again for your reviews, Benjamin (and others). I created issue 16799 for the next phase of this process: changing regrtest.main() from operating on getopt-style parsed options to an argparse Namespace object. -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: Anton, you are free to make that issue and propose a patch :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: New patch is LGTM. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: Chris, would you commit it? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: BTW, maybe someone should create an issue for full tests coverage of regrtest? I think such an issue would need to be a meta-issue because it is a very large task (involving many patches) and may never be truly finished. Chris, would you commit it? Sure, I should be able to get to it within a day or two. (By the way, after this is committed, the use of a Namespace return value can be phased in gradually as a replacement for some options in the getopt for loop. It need not be all or nothing.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: Agree with both points -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
R. David Murray added the comment: I'm still -0 on making a new file. regrtest will be running as __main__, so I don't see any reason the test file can't import it to test functions within it. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
R. David Murray added the comment: (I could, of course, be wrong :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Benjamin Peterson added the comment: Yes, please don't make something called regrtestlib.py. Some of us like the bash completetion to work with on a small prefix. :) -- nosy: +benjamin.peterson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Benjamin, do you not want a new file at all, or are you just asking for a different name? regrlib was the previous name unless you have another suggestion. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Benjamin Peterson added the comment: I think bitdancer's suggestion of leaving it in regrtest.py is a good one. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: I'm attaching an updated patch incorporating David and Benjamin's suggestion. (Thanks a lot for the feedback, by the way.) I also added a test for the help option and refactored to avoid having to use context managers to check sys.argv, sys.stderr, etc. -- Added file: http://bugs.python.org/file28448/issue-15302-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Here is an alternative patch with a cleaner diff (keeping the help-related strings at the top before the import statements). -- Added file: http://bugs.python.org/file28449/issue-15302-5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Benjamin Peterson added the comment: I posted some review comments. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Thanks. I replied. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: Well. Do you like to apply Chris patch and wait for next step appear? On Mon, Dec 24, 2012 at 2:33 AM, R. David Murray rep...@bugs.python.org wrote: R. David Murray added the comment: I am -1 on doing this as one big refactoring, unless tests are written for regrtest first. Incremental (over a period of weeks or months, so that the changes get some soak time each time) is I think acceptable even without tests, given that this is a test runner and not part of Python proper. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Do you like to apply Chris patch and wait for next step appear? Just to clarify, I think this should read apply Chris patch after updating/reviewing. A couple file renames are needed, and I noticed a typo in a docstring. Other changes may be needed since 5 months have elapsed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
R. David Murray added the comment: Yes, I apologize for not getting around to a review previously. Chris: why regrlib.py at all? Why isn't the code in regrtest.py? And I'm not clear on why there is a desire to have regrtest be a package. Did I miss that discussion? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: why regrlib.py at all? Why isn't the code in regrtest.py? It was for a few related reasons. It was primarily to make it easier to reason about testing regrtest, and to avoid at the outset any pitfalls that might arise from the circularity of regrtest testing itself. For example, the regrtest module seems to carry a fair bit of state and top-level code, whereas the library module being added would be stateless. It would also let us maintain an obvious line between what is tested and what is not. There was no discussion of creating a package. However, if we go this route I would foresee moving functionality from regrtest to the library module as we expand the amount of regrtest-support code under test. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: By the way, I am in the process of cleaning up the patch a bit. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Anton Kasyanov added the comment: So I see that there is no need in full refactoring now. BTW, maybe someone should create an issue for full tests coverage of regrtest? Then it would be much easier to make a full refactoring. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Here is an updated, improved patch. -- Added file: http://bugs.python.org/file28422/issue-15302-3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Anton Kasyanov added the comment: I've looked through the second patch and I'm not sure about how argparse usage was implemented here - parse_args() result is being converted to getopt-style list of (option, value) pairs. Is there any sense in using argparse this way? -- nosy: +a.kasyanov, asvetlov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: The reason in part is because of the lack of unit tests of regrtest (as commenters above have noted). By preserving the getopt interface, we can keep almost all of the untested code as is. You should view the patch as a first step in refactoring to use argparse. We can remove the conversion code and the main for loop in later steps. Note to committers: I was meaning to rename regrlib.py when I was working on this. Until regrtest-related code is in its own subpackage, I think the file name should begin with regrtest -- perhaps regrtester.py or regrtestlib.py. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Andrew Svetlov added the comment: Hi Chris. Today we had python sprint and I've guessed to Anton to refactor the patch in good way with properly setting default values from regrtest.main to argparse args. Then use proper argparse actions for manipulating that args. After all we can use Namespace object returned from argparse.parse() or argparse.parse_known_args() if needed as input for next processing. It will be big enough patch but I like to move it forward after double checking. regrtestlib.py name is good to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek added the comment: Yes, I agree with all of that but thought it would be easier to review if done incrementally as separate steps. In any case, I will look for Anton's patch on the review tool in case I have any comments. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
R. David Murray added the comment: I am -1 on doing this as one big refactoring, unless tests are written for regrtest first. Incremental (over a period of weeks or months, so that the changes get some soak time each time) is I think acceptable even without tests, given that this is a test runner and not part of Python proper. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Changes by Tshepang Lekhonkhobe tshep...@gmail.com: -- nosy: +tshepang ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Georg Brandl ge...@python.org added the comment: Please leave this for Python 3.4 -- it is not a bugfix. -- nosy: +georg.brandl versions: +Python 3.4 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek chris.jerdo...@gmail.com added the comment: Here is a patch that creates some unit tests for the existing getopt argument parsing code. In response to the comments, I'm thinking of a less invasive approach that involves wrapping argparse's parse_args() to return getopt-like output (and in particular, to match test expectations). In this way, we will be able to keep main() mostly unaffected while still being able to use argparse. -- keywords: +patch Added file: http://bugs.python.org/file26349/issue-15302-1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek chris.jerdo...@gmail.com added the comment: Attached is a first version of a complete patch. Note that I found three bugs in the current argument parsing code in the course of working on this patch: issue 15324, issue 15325, and issue 15326 (because of various typos in the getopt configuration). All of these should be fixed by the current patch. -- Added file: http://bugs.python.org/file26355/issue-15302-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Changes by Brett Cannon br...@python.org: -- stage: needs patch - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Senthil Kumaran sent...@uthcode.com added the comment: On Mon, Jul 9, 2012 at 8:50 PM, Chris Jerdonek rep...@bugs.python.org wrote: Sure, if someone is open to reviewing it. Yes, please go ahead. I can review it and I believe others should be able to as well. As a first short, I think, it may be a good idea to just replacing the short opt and long opt handling behavior and not dwell too much into argparse features (just to ascertain that we are not going through a new code-path. this I believe, may help assuage Ezio's concern a bit that regrtest.py lacks test and changes to them should be carefully done at this point in time in release). Or the other idea is you may submit the patch and if it is significant, it may go in early in 3.4 -- nosy: +orsenthil ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek chris.jerdo...@gmail.com added the comment: Thanks, Senthil. That is my plan. I should be able to have code with tests in no later than a week. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek chris.jerdo...@gmail.com added the comment: By the way, issue 15300 has a related patch that is ready to review today. Assuming that one is okay, it would make sense to commit first because it overlaps with the changes I'll be doing here. Issue 15305 is another related issue (also overlapping) that is probably worth discussing. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
R. David Murray rdmur...@bitdance.com added the comment: As mentioned, the first step is to create some tests that can validate the current behavior, so that changes don't break things. This is a non-trivial task. I know from experience with a similar refactoring that even seemingly simple changes can have unexpected consequences, and that getting good functional test coverage (not code-line test coverage) is hard. This is complicated by the fact that regrtest is an over-evolved mess. My ideal is to move appropriate pieces of the functionality into unittest and make regrtest a wrapper around that, but obviously I haven't spent much time actually doing that. I don't think that regrtest tests need to be run as part of the standard python test run, by the way, though I suppose they could be. -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Ezio Melotti ezio.melo...@gmail.com added the comment: Do you want to propose a patch? regrtest has no tests, so switching to argparse might cause more harm than good right now. -- nosy: +ezio.melotti stage: - needs patch type: - enhancement ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek chris.jerdo...@gmail.com added the comment: Sure, if someone is open to reviewing it. The parsing code doesn't seem to be doing anything too fancy right now. I can decouple the parsing code and begin adding tests around parts that may need it more. Increasing coverage will be easier going forward. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
New submission from Chris Jerdonek chris.jerdo...@gmail.com: I think it would be an improvement to switch from using getopt to argparse in test.regrtest. The code would be easier to maintain, it would give us more powerful options going forward, and it would improve the usability of the test command (e.g. nicer command-line help). -- components: Tests keywords: easy messages: 165064 nosy: cjerdonek priority: normal severity: normal status: open title: Use argparse instead of getopt in test.regrtest versions: Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15302] Use argparse instead of getopt in test.regrtest
Chris Jerdonek chris.jerdo...@gmail.com added the comment: It is also in the spirit of dogfooding. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15302 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
