Merge pull request #1796 from nicoddemus/fix-rootdir-common-ancestor

Fix rootdir common ancestor

Author: nicoddemus

CHANGELOG.rst

@@ -35,6 +35,10 @@
   deprecated but still present. Thanks to `@RedBeardCode`_ and `@tomviner`_
   for PR (`#1626`_).
 
+* Refined logic for determining the ``rootdir``, considering only valid
+  paths which fixes a number of issues: `#1594`_, `#1435`_ and `#1471`_.
+  Thanks to `@blueyed`_ and `@davehunt`_ for the PR.
+
 * Always include full assertion explanation. The previous behaviour was hiding
   sub-expressions that happened to be False, assuming this was redundant information.
   Thanks `@bagerard`_ for reporting (`#1503`_). Thanks to `@davehunt`_ and
@@ -70,11 +74,14 @@
 *
 
 .. _#1210: https://github.com/pytest-dev/pytest/issues/1210
+.. _#1435: https://github.com/pytest-dev/pytest/issues/1435
+.. _#1471: https://github.com/pytest-dev/pytest/issues/1471
 .. _#1479: https://github.com/pytest-dev/pytest/issues/1479
 .. _#1503: https://github.com/pytest-dev/pytest/issues/1503
 .. _#1553: https://github.com/pytest-dev/pytest/issues/1553
 .. _#1579: https://github.com/pytest-dev/pytest/issues/1579
 .. _#1580: https://github.com/pytest-dev/pytest/pull/1580
+.. _#1594: https://github.com/pytest-dev/pytest/issues/1594
 .. _#1597: https://github.com/pytest-dev/pytest/pull/1597
 .. _#1605: https://github.com/pytest-dev/pytest/issues/1605
 .. _#1626: https://github.com/pytest-dev/pytest/pull/1626

_pytest/config.py

@@ -1095,6 +1095,8 @@ def get_common_ancestor(args):
         if str(arg)[0] == "-":
             continue
         p = py.path.local(arg)
+        if not p.exists():
+            continue
         if common_ancestor is None:
             common_ancestor = p
         else:
@@ -1108,29 +1110,42 @@ def get_common_ancestor(args):
                     common_ancestor = shared
     if common_ancestor is None:
         common_ancestor = py.path.local()
-    elif not common_ancestor.isdir():
+    elif common_ancestor.isfile():
         common_ancestor = common_ancestor.dirpath()
     return common_ancestor
 
 
+def get_dirs_from_args(args):
+    return [d for d in (py.path.local(x) for x in args
+                        if not str(x).startswith("-"))
+            if d.exists()]
+
+
 def determine_setup(inifile, args):
+    dirs = get_dirs_from_args(args)
     if inifile:
         iniconfig = py.iniconfig.IniConfig(inifile)
         try:
             inicfg = iniconfig["pytest"]
         except KeyError:
             inicfg = None
-        rootdir = get_common_ancestor(args)
+        rootdir = get_common_ancestor(dirs)
     else:
-        ancestor = get_common_ancestor(args)
+        ancestor = get_common_ancestor(dirs)
         rootdir, inifile, inicfg = getcfg(
             [ancestor], ["pytest.ini", "tox.ini", "setup.cfg"])
         if rootdir is None:
             for rootdir in ancestor.parts(reverse=True):
                 if rootdir.join("setup.py").exists():
                     break
             else:
-                rootdir = ancestor
+                rootdir, inifile, inicfg = getcfg(
+                    dirs, ["pytest.ini", "tox.ini", "setup.cfg"])
+                if rootdir is None:
+                    rootdir = get_common_ancestor([py.path.local(), ancestor])
+                    is_fs_root = os.path.splitdrive(str(rootdir))[1] == os.sep
+                    if is_fs_root:
+                        rootdir = ancestor
     return rootdir, inifile, inicfg or {}
 
 

doc/en/customize.rst

@@ -29,25 +29,29 @@ project/testrun-specific information.
 
 Here is the algorithm which finds the rootdir from ``args``:
 
-- determine the common ancestor directory for the specified ``args``.
+- determine the common ancestor directory for the specified ``args`` that are
+  recognised as paths that exist in the file system. If no such paths are
+  found, the common ancestor directory is set to the current working directory.
 
-- look for ``pytest.ini``, ``tox.ini`` and ``setup.cfg`` files in the
-  ancestor directory and upwards.  If one is matched, it becomes the
-  ini-file and its directory becomes the rootdir.  An existing
-  ``pytest.ini`` file will always be considered a match whereas
-  ``tox.ini`` and ``setup.cfg`` will only match if they contain
-  a ``[pytest]`` section.
+- look for ``pytest.ini``, ``tox.ini`` and ``setup.cfg`` files in the ancestor
+  directory and upwards.  If one is matched, it becomes the ini-file and its
+  directory becomes the rootdir.
 
-- if no ini-file was found, look for ``setup.py`` upwards from
-  the common ancestor directory to determine the ``rootdir``.
+- if no ini-file was found, look for ``setup.py`` upwards from the common
+  ancestor directory to determine the ``rootdir``.
 
-- if no ini-file and no ``setup.py`` was found, use the already
-  determined common ancestor as root directory.  This allows to
-  work with pytest in structures that are not part of a package
-  and don't have any particular ini-file configuration.
+- if no ``setup.py`` was found, look for ``pytest.ini``, ``tox.ini`` and
+  ``setup.cfg`` in each of the specified ``args`` and upwards. If one is
+  matched, it becomes the ini-file and its directory becomes the rootdir.
 
-Note that options from multiple ini-files candidates are never merged,
-the first one wins (``pytest.ini`` always wins even if it does not
+- if no ini-file was found, use the already determined common ancestor as root
+  directory. This allows to work with pytest in structures that are not part of
+  a package and don't have any particular ini-file configuration.
+
+Note that an existing ``pytest.ini`` file will always be considered a match,
+whereas ``tox.ini`` and ``setup.cfg`` will only match if they contain a
+``[pytest]`` section. Options from multiple ini-files candidates are never
+merged - the first one wins (``pytest.ini`` always wins, even if it does not
 contain a ``[pytest]`` section).
 
 The ``config`` object will subsequently carry these attributes:

testing/test_collection.py

@@ -490,7 +490,8 @@ def test_method(self):
 class Test_getinitialnodes:
     def test_global_file(self, testdir, tmpdir):
         x = tmpdir.ensure("x.py")
-        config = testdir.parseconfigure(x)
+        with tmpdir.as_cwd():
+            config = testdir.parseconfigure(x)
         col = testdir.getnode(config, x)
         assert isinstance(col, pytest.Module)
         assert col.name == 'x.py'
@@ -504,7 +505,8 @@ def test_pkgfile(self, testdir):
         subdir = tmpdir.join("subdir")
         x = subdir.ensure("x.py")
         subdir.ensure("__init__.py")
-        config = testdir.parseconfigure(x)
+        with subdir.as_cwd():
+            config = testdir.parseconfigure(x)
         col = testdir.getnode(config, x)
         assert isinstance(col, pytest.Module)
         assert col.name == 'x.py'

testing/test_config.py

@@ -468,7 +468,8 @@ def test_consider_args_after_options_for_rootdir_and_inifile(testdir, args):
             args[i] = d1
         elif arg == 'dir2':
             args[i] = d2
-    result = testdir.runpytest(*args)
+    with root.as_cwd():
+        result = testdir.runpytest(*args)
     result.stdout.fnmatch_lines(['*rootdir: *myroot, inifile: '])
 
 
@@ -547,10 +548,14 @@ def test_hello(fix):
 class TestRootdir:
     def test_simple_noini(self, tmpdir):
         assert get_common_ancestor([tmpdir]) == tmpdir
-        assert get_common_ancestor([tmpdir.mkdir("a"), tmpdir]) == tmpdir
-        assert get_common_ancestor([tmpdir, tmpdir.join("a")]) == tmpdir
+        a = tmpdir.mkdir("a")
+        assert get_common_ancestor([a, tmpdir]) == tmpdir
+        assert get_common_ancestor([tmpdir, a]) == tmpdir
         with tmpdir.as_cwd():
             assert get_common_ancestor([]) == tmpdir
+            no_path = tmpdir.join('does-not-exist')
+            assert get_common_ancestor([no_path]) == tmpdir
+            assert get_common_ancestor([no_path.join('a')]) == tmpdir
 
     @pytest.mark.parametrize("name", "setup.cfg tox.ini pytest.ini".split())
     def test_with_ini(self, tmpdir, name):
@@ -595,3 +600,34 @@ def test_with_specific_inifile(self, tmpdir):
         inifile = tmpdir.ensure("pytest.ini")
         rootdir, inifile, inicfg = determine_setup(inifile, [tmpdir])
         assert rootdir == tmpdir
+
+    def test_with_arg_outside_cwd_without_inifile(self, tmpdir):
+        a = tmpdir.mkdir("a")
+        b = tmpdir.mkdir("b")
+        rootdir, inifile, inicfg = determine_setup(None, [a, b])
+        assert rootdir == tmpdir
+        assert inifile is None
+
+    def test_with_arg_outside_cwd_with_inifile(self, tmpdir):
+        a = tmpdir.mkdir("a")
+        b = tmpdir.mkdir("b")
+        inifile = a.ensure("pytest.ini")
+        rootdir, parsed_inifile, inicfg = determine_setup(None, [a, b])
+        assert rootdir == a
+        assert inifile == parsed_inifile
+
+    @pytest.mark.parametrize('dirs', ([], ['does-not-exist'],
+                                      ['a/does-not-exist']))
+    def test_with_non_dir_arg(self, dirs, tmpdir):
+        with tmpdir.ensure(dir=True).as_cwd():
+            rootdir, inifile, inicfg = determine_setup(None, dirs)
+            assert rootdir == tmpdir
+            assert inifile is None
+
+    def test_with_existing_file_in_subdir(self, tmpdir):
+        a = tmpdir.mkdir("a")
+        a.ensure("exist")
+        with tmpdir.as_cwd():
+            rootdir, inifile, inicfg = determine_setup(None, ['a/exist'])
+            assert rootdir == tmpdir
+            assert inifile is None