Repository: tox-dev/tox
Pull request #1171 (2019-02-22 00:10:24)

toxworkdir: use realpath
Fixes https://github.com/tox-dev/tox/issues/1169.

blueyed: Done.

Hmm, I've seen a CI failure, but it appears to be good now?!
https://toxdev.visualstudio.com/tox/_build/results?buildId=1218&view=logs&jobId=5c9cfffa-02af-5931-d194-60c5bf721ca9&taskId=355263bf-342e-5ce0-d70e-7a006ec8e0b4&lineStart=1042&lineEnd=1084&colStart=1&colEnd=77
blueyed: Ok, now the status shows up as "failed".  For linux_py27.
Any idea?  Works locally.

```
__________________ TestParseconfig.test_workdir_gets_resolved __________________
[gw0] linux2 -- Python 2.7.15 /home/vsts/work/1/s/.tox/py27/bin/python

self = <test_config.TestParseconfig instance at 0x7ff435f96ea8>
tmpdir = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0')

    @pytest.mark.skipif(sys.platform == "win32", reason="no symlinks on Windows")
    def test_workdir_gets_resolved(self, tmpdir):
        """
        Test explicitly setting config path, both with and without the filename
        """
        real = tmpdir.mkdir("real")
        symlink = tmpdir.join("link")
        symlink.mksymlinkto(real)
    
>       config = parseconfig(["--workdir", str(symlink)])

real       = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0/real')
self       = <test_config.TestParseconfig instance at 0x7ff435f96ea8>
symlink    = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0/link')
tmpdir     = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0')

/home/vsts/work/1/s/tests/unit/config/test_config.py:291: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/home/vsts/work/1/s/.tox/py27/lib/python2.7/site-packages/tox/config/__init__.py:245: in parseconfig
    feedback(msg.format(candidates), sysexit=not (option.help or option.helpini))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

msg = 'tox config file (either pyproject.toml, tox.ini, setup.cfg) not found'
sysexit = True

    def feedback(msg, sysexit=False):
        print("ERROR: {}".format(msg), file=sys.stderr)
        if sysexit:
>           raise SystemExit(1)
E           SystemExit: 1

msg        = 'tox config file (either pyproject.toml, tox.ini, setup.cfg) not found'
sysexit    = True

/home/vsts/work/1/s/.tox/py27/lib/python2.7/site-packages/tox/config/__init__.py:294: SystemExit
----------------------------- Captured stderr call -----------------------------
ERROR: tox config file (either pyproject.toml, tox.ini, setup.cfg) not found
=============================== warnings summary ===============================
```
blueyed: 1. Shouldn't it fail on all linux jobs?
2. Shouldn't it find the `tox.ini` from tox itself?  Is this some race where cwd is something else?!

Re-triggered a build, where it also failed for pypy.
Will use `--help` to work around this, but it is not clear to me why it is necessary.
gaborbernat: Yes probably, we ran tests in parallel, so the cwd is non deterministic (each test should ensure itself the environment is correctly set-up - some bad tests might not reset their changes).
blueyed: Ok, thanks for the info.

> some bad tests might not reset their changes

Could be ensured maybe with an autouse fixture.
gaborbernat: Neah to many moving things. Tests should be declarative, and define upfront if they want to really on some stuff (e.g. tox.ini from project). Explicit over implicit.
blueyed: Well.. tests should also not change the environment, right?  (Why is `monkeypatch.chdir` used in some places then?)
blueyed: Why is this now?  (only linux_py36)

```
__________________ TestParseconfig.test_workdir_gets_resolved __________________
[gw0] linux -- Python 3.6.5 /home/vsts/work/1/s/.tox/py36/bin/python

self = <test_config.TestParseconfig object at 0x7f06c7614518>
tmpdir = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0')

    @pytest.mark.skipif(sys.platform == "win32", reason="no symlinks on Windows")
    def test_workdir_gets_resolved(self, tmpdir):
        """
        Test explicitly setting config path, both with and without the filename
        """
        real = tmpdir.mkdir("real")
        symlink = tmpdir.join("link")
        symlink.mksymlinkto(real)
    
        config = parseconfig(["--workdir", str(symlink), "--help"])
>       assert config.toxworkdir == real
E       AttributeError: 'Config' object has no attribute 'toxworkdir'

config     = <tox.config.Config object at 0x7f06c736cf28>
real       = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0/real')
self       = <test_config.TestParseconfig object at 0x7f06c7614518>
symlink    = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0/link')
tmpdir     = local('/home/vsts/work/1/b/pytest-of-vsts/pytest-0/popen-gw0/test_workdir_gets_resolved0')

/home/vsts/work/1/s/tests/unit/config/test_config.py:292: AttributeError
----------------------------- Captured stderr call -----------------------------
ERROR: tox config file (either pyproject.toml, tox.ini, setup.cfg) not found
```
gaborbernat: we try to clean up after us as much as possible, however much of the tests were written way before this best practice was established☹️ and tox itself changes many states (environment/cwd), so some tests inadvertently miss some of this even if at test level all looks good
blueyed: Should be good now I guess.
Please squash-merge then.
gaborbernat: looks good, can you switch to tmp_path please? thanks!
gaborbernat: we'll also need a changelog entry to warn users about this in case this will cause issues

Patch:
diff --git a/docs/changelog/1169.bugfix.rst b/docs/changelog/1169.bugfix.rst
new file mode 100644
index 00000000..7079b1fd
--- /dev/null
+++ b/docs/changelog/1169.bugfix.rst
@@ -0,0 +1,1 @@
+Resolve symlinks with ``toxworkdir`` - by :user:`blueyed`.
diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py
index 15627235..66612581 100644
--- a/src/tox/config/__init__.py
+++ b/src/tox/config/__init__.py
@@ -999,6 +999,9 @@ class ParseIni(object):
         else:
             config.toxworkdir = config.toxinidir.join(config.option.workdir, abs=True)
 
+        if os.path.exists(str(config.toxworkdir)):
+            config.toxworkdir = config.toxworkdir.realpath()
+
         if config.option.skip_missing_interpreters == "config":
             val = reader.getbool("skip_missing_interpreters", False)
             config.option.skip_missing_interpreters = "true" if val else "false"


Test patch:
diff --git a/src/tox/_pytestplugin.py b/src/tox/_pytestplugin.py
index 651ba976..b297fa46 100644
--- a/src/tox/_pytestplugin.py
+++ b/src/tox/_pytestplugin.py
@@ -49,6 +49,15 @@ def work_in_clean_dir(tmpdir):
         yield
 
 
+@pytest.fixture(autouse=True)
+def check_cwd_not_changed_by_test():
+    old = os.getcwd()
+    yield
+    new = os.getcwd()
+    if old != new:
+        pytest.fail("test changed cwd: {!r} => {!r}".format(old, new))
+
+
 @pytest.fixture(name="newconfig")
 def create_new_config_file(tmpdir):
     def create_new_config_file_(args, source=None, plugins=()):
@@ -350,7 +359,8 @@ def initproj(tmpdir):
         base.chdir()
         return base
 
-    return initproj_
+    with py.path.local().as_cwd():
+        yield initproj_
 
 
 def _path_parts(path):
diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py
index ed29a5e2..02aff798 100644
--- a/tests/unit/config/test_config.py
+++ b/tests/unit/config/test_config.py
@@ -236,8 +236,8 @@ class TestConfigPackage:
         assert config.distshare == config.homedir.join(".tox", "distshare")
 
     def test_defaults_changed_dir(self, tmpdir, newconfig):
-        tmpdir.mkdir("abc").chdir()
-        config = newconfig([], "")
+        with tmpdir.mkdir("abc").as_cwd():
+            config = newconfig([], "")
         assert config.setupdir.realpath() == tmpdir.realpath()
         assert config.toxworkdir.realpath() == tmpdir.join(".tox").realpath()
 
@@ -257,11 +257,8 @@ class TestParseconfig:
     def test_search_parents(self, tmpdir):
         b = tmpdir.mkdir("a").mkdir("b")
         toxinipath = tmpdir.ensure("tox.ini")
-        old = b.chdir()
-        try:
+        with b.as_cwd():
             config = parseconfig([])
-        finally:
-            old.chdir()
         assert config.toxinipath == toxinipath
 
     def test_explicit_config_path(self, tmpdir):
@@ -279,6 +276,21 @@ class TestParseconfig:
         config = parseconfig(["-c", str(path)])
         assert config.toxinipath == config_file_path
 
+    @pytest.mark.skipif(sys.platform == "win32", reason="no symlinks on Windows")
+    def test_workdir_gets_resolved(self, tmp_path, monkeypatch):
+        """
+        Test explicitly setting config path, both with and without the filename
+        """
+        real = tmp_path / "real"
+        real.mkdir()
+        symlink = tmp_path / "link"
+        symlink.symlink_to(real)
+
+        (tmp_path / "tox.ini").touch()
+        monkeypatch.chdir(tmp_path)
+        config = parseconfig(["--workdir", str(symlink)])
+        assert config.toxworkdir == real
+
 
 def test_get_homedir(monkeypatch):
     monkeypatch.setattr(py.path.local, "_gethomedir", classmethod(lambda x: {}[1]))
diff --git a/tests/unit/test_z_cmdline.py b/tests/unit/test_z_cmdline.py
index 7ade17aa..e61a2d61 100644
--- a/tests/unit/test_z_cmdline.py
+++ b/tests/unit/test_z_cmdline.py
@@ -441,10 +441,10 @@ def test_toxuone_env(cmd, example123):
     )
 
 
-def test_different_config_cwd(cmd, example123, monkeypatch):
+def test_different_config_cwd(cmd, example123):
     # see that things work with a different CWD
-    monkeypatch.chdir(example123.dirname)
-    result = cmd("-c", "example123/tox.ini")
+    with example123.dirpath().as_cwd():
+        result = cmd("-c", "example123/tox.ini")
     assert not result.ret
     assert re.match(
         r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*",
@@ -522,7 +522,7 @@ def test_usedevelop_mixed(initproj, cmd):
 
 @pytest.mark.parametrize("skipsdist", [False, True])
 @pytest.mark.parametrize("src_root", [".", "src"])
-def test_test_usedevelop(cmd, initproj, src_root, skipsdist, monkeypatch):
+def test_test_usedevelop(cmd, initproj, src_root, skipsdist):
     name = "example123-spameggs"
     base = initproj(
         (name, "0.5"),
@@ -564,16 +564,15 @@ def test_test_usedevelop(cmd, initproj, src_root, skipsdist, monkeypatch):
     )
 
     # see that things work with a different CWD
-    monkeypatch.chdir(base.dirname)
-    result = cmd("-c", "{}/tox.ini".format(name))
-    assert not result.ret
-    assert "develop-inst-noop" in result.out
-    assert re.match(
-        r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*",
-        result.out,
-        re.DOTALL,
-    )
-    monkeypatch.chdir(base)
+    with base.dirpath().as_cwd():
+        result = cmd("-c", "{}/tox.ini".format(name))
+        assert not result.ret
+        assert "develop-inst-noop" in result.out
+        assert re.match(
+            r".*\W+1\W+passed.*" r"summary.*" r"python:\W+commands\W+succeeded.*",
+            result.out,
+            re.DOTALL,
+        )
 
     # see that tests can also fail and retcode is correct
     testfile = py.path.local("tests").join("test_hello.py")