classification
Title: Refactor package creation support code into a common location
Type: Stage: needs patch
Components: Tests Versions: Python 3.3
process
Status: open Resolution:
Dependencies: 15494 Superseder:
Assigned To: ncoghlan Nosy List: chris.jerdonek, eric.araujo, eric.snow, ncoghlan, pitrou
Priority: normal Keywords: easy, patch

Created on 2012-07-20 11:13 by chris.jerdonek, last changed 2012-11-13 05:04 by eric.snow.

Files
File name Uploaded Description Edit
issue-15403-1.patch chris.jerdonek, 2012-07-20 11:26 review
issue-15403-2.patch chris.jerdonek, 2012-08-02 22:23 review
Messages (17)
msg165910 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-20 11:13
This issue addresses the "file creation" portion of issue 15376, which is to refactor the walk_package support code in test_runpy into a common location.
msg165912 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-20 11:26
Attaching patch.
msg166499 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-26 18:58
It occurs to me that the filecmp/dircmp tests in Lib/test/test_filecmp.py would also benefit from code like this (i.e. being able to create a nested directory of files in one or two lines).  And perhaps elsewhere in the tests.

This is an argument for slightly generalizing the test support API in the uploaded patch from creating modules/packages to creating files/directories.  A convenience wrapper for packages could still be included that includes an __init__.py in the calls to the underlying directory code.
msg166759 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-29 13:36
Lib/test/test_import.py also contains test code that would benefit from this (see for example issue 15425).  (Though not all files need to be refactored in a single issue.)
msg166771 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-29 14:54
Please let's not have something called test.supportlib in addition to test.support. If test.support grows too large we can turn it into a package.
msg166773 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-29 15:03
I feel like it is already too large (it is over 1750 lines), and I did not want to create a third sibling test support module (there is also test/script_helper.py that overlaps with test.support).  Do you think that the community would be open to refactoring test.support into a package for Python 3.3?  This was meant to assist in increasing test coverage for Python 3.3 bugs.
msg166774 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-29 15:14
> I feel like it is already too large (it is over 1750 lines), and I did
> not want to create a third sibling test support module (there is also
> test/script_helper.py that overlaps with test.support).  Do you think
> that the community would be open to refactoring test.support into a
> package for Python 3.3?  This was meant to assist in increasing test
> coverage for Python 3.3 bugs.

That's too late for 3.3, IMO. Whether or not test.support is too large
is a matter of taste (we have larger files in the stdlib; what makes
test.support annoying to work with is that it's a hodge-podge of utility
functions with little care for consistency). Unless we can clearly
separate it in thematic areas, making it a package would only make
finding stuff more difficult.
msg166776 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-29 15:23
At this point, would you advise me to add even more to the existing hodge podge, or to create a third sibling test support module?  My patch adds closely related test support functionality.

Incidentally, this discussion relates to the point I was getting at in the python-dev thread in the past couple days about how restrictions on refactoring test support code for maintenance releases can affect the quality of our test code.  I am still struggling with how to approach that.
msg166778 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-29 15:40
I don't mind refactoring test support routines in maintenance release. I
said it was too late for 3.3 because the beta period is closing; but it
could be fine for 3.3.1 ;)

As for the way forward, I see three possibilites:
1) put everying in test.support, as a single module (that includes
folding script_helper into it)
2) make test.support a package of thematic modules
3) refactor test.support into a variety of independent modules
(script_helper, import_helper, socket_helper, etc.); a small
test.support providing fundamental features such as TESTFN could remain

My personal preference would be for 1) or 3). But whatever we choose, we
should do it consistently. Having a large test.support + a supportlib
package + some independent helpers is not a good idea.
msg166787 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-29 16:54
Thanks for your thoughts.  For the purposes of this patch, I will change to putting the new support functionality in test.support.  Going forward, if we could do some of the refactoring for 3.3.1, that would be great. :)

I worry that the third option may make things worse because it would become less obvious where all of the different test support functionality is located.

Are you opposed to (2), or is it simply less favorable?  The transition to (2) could be a gradual one beginning with two or even one module inside the package.
msg166788 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-29 16:56
> Are you opposed to (2), or is it simply less favorable?

Less favorable, because it produces longer import strings
("test.support.some_helper" instead of "test.some_helper").
msg166790 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-29 17:00
> Less favorable, because it produces longer import strings
("test.support.some_helper" instead of "test.some_helper").

This can be addressed by exposing the API in __init__.py though (as does, say, the unittest package), no?
msg166793 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-29 17:10
Le dimanche 29 juillet 2012 à 17:00 +0000, Chris Jerdonek a écrit :
> Chris Jerdonek added the comment:
> 
> > Less favorable, because it produces longer import strings
> ("test.support.some_helper" instead of "test.some_helper").
> 
> This can be addressed by exposing the API in __init__.py though (as
> does, say, the unittest package), no?

Yes, that's right.
msg166844 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-07-30 01:19
Discoverability is definitely a problem - part of that is a docs issue, since test.support is currently the only one mentioned in the prose docs.

One advantage to moving to a support subpackage is that we can lose the "helper" suffix from the names, while still allowing thematic divisions.

So, for example, we could have

# support.py -> support/__init__.py
import test.support
# script_helper.py -> support/scripts.py
import test.support.scripts
# Other possible additions
import test.support.imports
import test.support.network

Even without updating the prose docs, this would help discoverability a lot by having a much smaller directory listing to scan for useful support code. At the moment the general purpose helper modules are mixed in with the tests

I agree with Antoine that we're probably better off handling this as a refactor post-release at this point, though. I'd hoped to have the time to devote to it beforehand, but there's user facing stuff that's higher priority right now.
msg166847 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-30 01:33
Sounds good.  Later today I will create an issue to move test/support.py into a test.support subpackage post-release.

We can continue the discussion of how to organize it there.
msg166850 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-30 01:42
> Later today I will create an issue to move test/support.py into a test.support subpackage post-release.

I created issue 15494 for this.
msg167266 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-02 22:23
Attaching an updated patch.  Changes include:

1. Cleans up and expands support API to include creating nested directory structures (for use in the filecmp tests, for example).
2. Adds unit tests for the full API (similar to test_support.py).
3. Refactors test_filecmp and test_import to use the support code, in addition to test_runpy and test_cmd_line_script. We can remove some of these refactorings if you think it does not add much at this point.
4. Puts changes in package_helper.py for now (until issue 15494 is done). I didn't put them in script_helper.py to avoid creating a circular dependency.
History
Date User Action Args
2012-11-13 05:04:09eric.snowsetnosy: + eric.snow
2012-08-02 22:23:28chris.jerdoneksetfiles: + issue-15403-2.patch

messages: + msg167266
2012-07-30 10:26:40ncoghlansetdependencies: + Move test/support.py into a test.support subpackage
2012-07-30 01:42:37chris.jerdoneksetmessages: + msg166850
2012-07-30 01:33:08chris.jerdoneksetmessages: + msg166847
2012-07-30 01:19:02ncoghlansetmessages: + msg166844
2012-07-29 17:10:07pitrousetmessages: + msg166793
2012-07-29 17:00:46chris.jerdoneksetmessages: + msg166790
2012-07-29 16:56:13pitrousetmessages: + msg166788
2012-07-29 16:54:26chris.jerdoneksetmessages: + msg166787
2012-07-29 15:40:29pitrousetmessages: + msg166778
2012-07-29 15:23:36chris.jerdoneksetmessages: + msg166776
2012-07-29 15:14:09pitrousetmessages: + msg166774
2012-07-29 15:03:38chris.jerdoneksetmessages: + msg166773
2012-07-29 14:54:39pitrousetnosy: + pitrou
messages: + msg166771
2012-07-29 13:36:46chris.jerdoneksetmessages: + msg166759
2012-07-26 18:58:10chris.jerdoneksetmessages: + msg166499
2012-07-26 02:18:25ncoghlansetassignee: ncoghlan
2012-07-22 23:19:40eric.araujosetnosy: + eric.araujo
2012-07-20 11:26:15chris.jerdoneksetfiles: + issue-15403-1.patch
keywords: + patch
messages: + msg165912
2012-07-20 11:25:41chris.jerdoneksetnosy: + ncoghlan
2012-07-20 11:19:09chris.jerdoneklinkissue15376 dependencies
2012-07-20 11:13:12chris.jerdonekcreate