fix 4425: resolve --basetemp to absolute paths #4427
Conversation
RonnyPfannschmidt
force-pushed
the
fix-4425
branch
from
cafe9a7
to
63ae448
Compare
Codecov Report
@@ Coverage Diff @@
## master #4427 +/- ##
=========================================
- Coverage 95.9% 95.9% -0.01%
=========================================
Files 111 111
Lines 25008 25027 +19
Branches 2438 2442 +4
=========================================
+ Hits 23984 24001 +17
- Misses 723 724 +1
- Partials 301 302 +1
Continue to review full report at Codecov.
|
| @@ -25,12 +26,29 @@ def test_ensuretemp(recwarn): | |||
| assert d1.check(dir=1) | |||
|
|
|||
|
|
|||
| @attr.s | |||
| class FakeConfig(object): | |||
There was a problem hiding this comment.
TBH not sure if this fake actually improves things... the "option returns self" and get are quite the hack. 😬
There was a problem hiding this comment.
its indeed quite a hack, we cant make a sane config object for use in such cases
| @@ -40,6 +58,14 @@ def test_mktemp(self, testdir): | |||
| assert tmp2.relto(t.getbasetemp()).startswith("this") | |||
| assert tmp2 != tmp | |||
|
|
|||
| def test_tmppath_relative_basetemp_absolute(self, tmp_path, monkeypatch): | |||
| from _pytest.tmpdir import TempPathFactory | |||
There was a problem hiding this comment.
I would add a docstring and a reference to the bug. 👍
RonnyPfannschmidt
force-pushed
the
fix-4425
branch
from
63ae448
to
fc61bdd
Compare
Unfortunately it seems there is a difference in resolve() behavior depending on the platform
…tance There's Path.absolute(), but it is not public, see https://bugs.python.org/issue25012.
|
Found a discussion about Fix the |
|
restarted the build, travis was broken |
|
Weird, on MacOS the test fails, here's the exception formatted for readability: PosixPath('/private/var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/pytest-of-travis/pytest-0/test_tmppath_relative_basetemp0/hello')
==
PosixPath('/var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/pytest-of-travis/pytest-0/test_tmppath_relative_basetemp0') / 'hello'Seems to be symlink-related... |
|
it is symlink related, other projects see the same issue we can use resolve on the resulting paths |
|
ahh good catch! |
|
Hello. I am looking forward to this fix. Which release is it going to be in? Thanks. |
|
the next release in any case |
|
Just to be clear... 4.0.1? |
|
yes |
| # does not work the same in all platforms (see #4427) | ||
| # Path.absolute() exists, but it is not public (see https://bugs.python.org/issue25012) | ||
| convert=attr.converters.optional( | ||
| lambda p: Path(os.path.abspath(six.text_type(p))) |
There was a problem hiding this comment.
Brief note (was stalking IRC so I saw this comment) -- resolve() doesn't only resolve symlinks, but on windows it also resolves UNC paths from mounts (e.g. if you mount a network share at R:\\Some\\path but it really points at \\10.1.100.100\Some\path, Path.resolve() captures that but os.path.abspath() will return R:\\blah -- despite absolute not being public we typically handle this by trying resolve and falling back on absolute in pipenv at least
There was a problem hiding this comment.
Thanks @techalchemy for the note, that's interesting! 👍
fixes #4425