Make tmpdir more resilient in case environment variables required by getpass are missing #1013
Conversation
…getpass are missing Fix pytest-dev#1010
|
cc @alex |
|
please move the username getting into a utility function and unit-test that |
Good idea, done. |
| temproot = py.path.local.get_temproot() | ||
| rootdir = temproot.join('pytest-of-%s' % getpass.getuser()) | ||
| rootdir = temproot.join('pytest-of-%s' % get_user()) |
There was a problem hiding this comment.
That actually looks odd to me in combination with using a fixed pytest-tox. If you have a Jenkins machine that executes multiple tox runs is this not bound to lead to bad interactions?
What about simply not creating a subdir but using the "slow" make_numbered_dir on the temproot?
There was a problem hiding this comment.
If you have a Jenkins machine that executes multiple tox runs is this not bound to lead to bad interactions?
Hmm I don't think so, because py.path.local.make_numbered_dir is atomic, and will correctly create a separate directory for each invocation... pytest-tox will only be used when running py.test from inside tox without passing %USERNAME on Windows.
What about simply not creating a subdir but using the "slow" make_numbered_dir on the temproot?
That's how the code was written originally, but it was changed as an optimization. From the CHANGELOG:
- optimized tmpdir fixture initialization, which should make test sessions
faster (specially when using pytest-xdist). The only visible effect
is that now pytest uses a subdirectory in the $TEMP directory for all
directories created by this fixture (defaults to $TEMP/pytest-$USER).
There was a problem hiding this comment.
sorry i was unclear. I meant not creating a subdir if we don't have a username.
Suppose two users on a Windows machine where each executes tox which executes pytest. Will they not conflict on creating pytest-tox at the temproot by default?
There was a problem hiding this comment.
sorry i was unclear. I meant not creating a subdir if we don't have a username.
Oh I see, thanks for clarifying.
Suppose two users on a Windows machine where each executes tox which executes pytest. Will they not conflict on creating pytest-tox at the temproot by default?
Hmm I don't see how this could be a problem... make_numbered_dir will guarantee that each process gets its own unique directory, the fact that they share the same rootdir shouldn't matter.
There was a problem hiding this comment.
If two processes from different accounts concurrently run without a USERNAME setting then this ensure or the subsequent make_numbered_dir calls can run into permission problems, no?
Oh that's a good point! 😅
I will follow your advice and fallback to not using a root directory at all if there's no user name.
|
FWIW, an alternate solution might be to just have |
| temproot = py.path.local.get_temproot() | ||
| rootdir = temproot.join('pytest-of-%s' % getpass.getuser()) | ||
| rootdir = temproot.join('pytest-of-%s' % get_user()) | ||
| rootdir.ensure(dir=1) |
There was a problem hiding this comment.
If two processes from different accounts concurrently run without a USERNAME setting then this ensure or the subsequent make_numbered_dir calls can run into permission problems, no?
|
Alex: IIRC not having a username can even appear in non-tox runs (debian or some other build sandboxed environments). Also people might be running mixtures of old/new tox/pytest versions given they are both deployed widely. So i think it makes sense to fix it here and independently consider tox adding USERNAME/USER to passenv. |
|
Ok, fair enough! On Wed, Sep 16, 2015 at 3:30 PM, hpk42 [email protected] wrote:
"I disapprove of what you say, but I will defend to the death your right to |
Make tmpdir more resilient in case environment variables required by getpass are missing
Fix #1010