It seems like our test harness is disambiguating more than it needs to for parallel testing.

In Lib/test/regrtest.py, we do this--

# Define a writable temp dir that will be used as cwd while running
# the tests. The name of the dir includes the pid to allow parallel
# testing (see the -j option).
TESTCWD = 'test_python_{}'.format(os.getpid())
...
with support.temp_cwd(TESTCWD, quiet=True):
    main()

And then in Lib/test/support.py, we are doing this--

# Disambiguate TESTFN for parallel testing, while letting it remain a valid
# module name.
TESTFN = "{}_{}_tmp".format(TESTFN, os.getpid())

with uses like--

with open(TESTFN, "wb") as f:
    # Do stuff with f.

It seems like only one of these measures should be necessary (a single working directory for all parallel tests using a disambiguated TESTFN, or one working directory for each process with a non-disambiguated TESTFN).

The former option seems to make more sense to me (a single working directory for all parallel tests using a disambiguated TESTFN).

Simplify to single build/test directory, and disambiguate by TEMPFN.

Test suite run on Mac OS X (./python.exe -m test -j3) without error. Some files created by tests do not use TESTFN, so may have build bots identify issues.

Looks good to me, and all tests also pass on my Ubuntu 12.10.

Chris: Would you be willing to commit this and watch the buildbots do their job? Or do the buildbots even use the -j option?

I would be happy to commit and watch the buildbots, once I have confidence in the patch though.  Question: I noticed that the following was changed in Lib/test/regrtest.py:

-    with support.temp_cwd(TESTCWD, quiet=True):
+    with support.temp_cwd(quiet=True, path=TESTCWD):

But the corresponding change wasn't made in Lib/test/__main__.py (which I believe is the code path used by Geoff's `./python.exe -m test -j3` invocation):

http://hg.python.org/cpython/file/96f08a22f562/Lib/test/__main__.py#l12

Those two code chunks should really share code by the way (even the code comment is copied verbatim), which would help in not needing to update code in two places as in this issue/patch.  Perhaps that should even be done first as part of a separate issue (to separate this into smaller changes).

> Those two code chunks should really share code by the way

I created issue 17283 for this (it's okay to fix the current issue before this one).

Both are called at different points when running './python.exe -m test -j3':
1. In __main__.py, this is called once at the start of the test run. By putting TESTCWD as the first/name arg, the directory is created.
2. In regrtest.py, the main file is called per test.

The support.temp_cwd command was raising a warning if the directory already existed. I've updated this to no longer raise a warning, although it will still try to create it. Passing it in explicitly as the path= argument would have it not try to create the directory.

I don't think support.temp_cwd() should be changed for this issue (or needs to be).  Also, changing it in the proposed way could mask errors in the test suite since tests were written against the current behavior.

regrtest.py and __main__.py should both behave the same with respect to creating the temp dir, etc. since both invocations should work from the command-line:

http://hg.python.org/cpython/file/96f08a22f562/Lib/test/regrtest.py#l9

The fix for issue 17283 has been committed now, which should make this slightly easier to fix (e.g. change one place instead of two).

Here is a patch that updates Geoff's patch to the latest code, and addresses the directory creation issue.

The patch is very short so can we have a formal review please.

I'm working on this in the PyCon 2019 sprints.

Near as I can tell, while this issue still seems relevant, I think it might actually be for the best that this multiple disambiguation is left in the test suite. 

I removed the pid reference in the TESTFN and the tests passed in both the parallel and sequential cases. However, removing the pid results in multiprocessing tests having to be written more carefully if they choose to use the TESTFN.

Here is an explanation for why you would leave this code in. When running the tests in sequential mode, the tests will run in a CWD that includes the pid. When writing a multi-processing test using the TESTFN you would have to remember to add the os.getpid call to the actual TESTFN instead of it just being included by default.

Ultimately, this is really just extra information that is included in the temporary filenames. It might be worth just closing this issue.

messages: + msg183172

messages: + msg182876

nosy: + gmwils
messages: + msg182752

keywords: + patch