From 64391f0411d495bb6f1f174a479ed6ba6fd7d35e Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 00:11:47 +0100 Subject: [PATCH 1/8] tests: re-harden test_cmdline_python_package_symlink This was done on purpose in 7268462b3, and then reverted in a non-merge "Merge" commit (wrong conflict resolution?): https://github.com/pytest-dev/pytest/pull/4158/commits/9646a1cd7#diff-1f1fc4eef6bf4c4da558e0dce9e74e9bR761-R762 --- testing/acceptance_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 4cbaebeb1a4..8f645d578da 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -830,8 +830,8 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): else: result.stdout.fnmatch_lines( [ - "*lib/foo/bar/test_bar.py::test_bar PASSED*", - "*lib/foo/bar/test_bar.py::test_other PASSED*", + "local/lib/foo/bar/test_bar.py::test_bar PASSED*", + "local/lib/foo/bar/test_bar.py::test_other PASSED*", "*2 passed*", ] ) From 15cea2ece435c63f69884b4e1b46ea22d3fa5ac2 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 01:05:01 +0100 Subject: [PATCH 2/8] remove skipif: it uses os.symlink anyway --- testing/acceptance_test.py | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 8f645d578da..543da402820 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -268,10 +268,6 @@ def test_conftest_printing_shows_if_error(self, testdir): assert result.ret != 0 assert "should be seen" in result.stdout.str() - @pytest.mark.skipif( - not hasattr(py.path.local, "mksymlinkto"), - reason="symlink not available on this platform", - ) def test_chdir(self, testdir): testdir.tmpdir.join("py").mksymlinkto(py._pydir) p = testdir.tmpdir.join("main.py") @@ -819,22 +815,13 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): result = testdir.runpytest("--pyargs", "-v", "foo.bar") testdir.chdir() assert result.ret == 0 - if hasattr(py.path.local, "mksymlinkto"): - result.stdout.fnmatch_lines( - [ - "lib/foo/bar/test_bar.py::test_bar PASSED*", - "lib/foo/bar/test_bar.py::test_other PASSED*", - "*2 passed*", - ] - ) - else: - result.stdout.fnmatch_lines( - [ - "local/lib/foo/bar/test_bar.py::test_bar PASSED*", - "local/lib/foo/bar/test_bar.py::test_other PASSED*", - "*2 passed*", - ] - ) + result.stdout.fnmatch_lines( + [ + "local/lib/foo/bar/test_bar.py::test_bar PASSED*", + "local/lib/foo/bar/test_bar.py::test_other PASSED*", + "*2 passed*", + ] + ) def test_cmdline_python_package_not_exists(self, testdir): result = testdir.runpytest("--pyargs", "tpkgwhatv") From 915e0b00b39ef010164574404714b6c53c938b17 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 01:24:38 +0100 Subject: [PATCH 3/8] fixup! remove skipif: it uses os.symlink anyway --- testing/acceptance_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 543da402820..a16621f5109 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -817,8 +817,8 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): assert result.ret == 0 result.stdout.fnmatch_lines( [ - "local/lib/foo/bar/test_bar.py::test_bar PASSED*", - "local/lib/foo/bar/test_bar.py::test_other PASSED*", + "lib/foo/bar/test_bar.py::test_bar PASSED*", + "lib/foo/bar/test_bar.py::test_other PASSED*", "*2 passed*", ] ) From 3d3f0658a499fa1349135407779c481c2b71761a Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 01:35:18 +0100 Subject: [PATCH 4/8] fixup! remove skipif: it uses os.symlink anyway --- testing/acceptance_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index a16621f5109..37ae185ad78 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -268,6 +268,10 @@ def test_conftest_printing_shows_if_error(self, testdir): assert result.ret != 0 assert "should be seen" in result.stdout.str() + @pytest.mark.skipif( + not hasattr(py.path.local, "mksymlinkto"), + reason="symlink not available on this platform", + ) def test_chdir(self, testdir): testdir.tmpdir.join("py").mksymlinkto(py._pydir) p = testdir.tmpdir.join("main.py") From bc92ff1db65f954ec2aedb27aee611e4e4078d39 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 01:56:12 +0100 Subject: [PATCH 5/8] factor out symlink_or_skip --- testing/acceptance_test.py | 16 ++++------------ testing/conftest.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 37ae185ad78..46f88ab6462 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -762,21 +762,13 @@ def test(): result = testdir.runpytest(str(p) + "::test", "--doctest-modules") result.stdout.fnmatch_lines(["*1 passed*"]) - def test_cmdline_python_package_symlink(self, testdir, monkeypatch): + def test_cmdline_python_package_symlink( + self, testdir, monkeypatch, symlink_or_skip + ): """ test --pyargs option with packages with path containing symlink can have conftest.py in their package (#2985) """ - # dummy check that we can actually create symlinks: on Windows `os.symlink` is available, - # but normal users require special admin privileges to create symlinks. - if sys.platform == "win32": - try: - os.symlink( - str(testdir.tmpdir.ensure("tmpfile")), - str(testdir.tmpdir.join("tmpfile2")), - ) - except OSError as e: - pytest.skip(str(e.args[0])) monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False) dirname = "lib" @@ -794,7 +786,7 @@ def test_cmdline_python_package_symlink(self, testdir, monkeypatch): d_local = testdir.mkdir("local") symlink_location = os.path.join(str(d_local), "lib") - os.symlink(str(d), symlink_location, target_is_directory=True) + symlink_or_skip(str(d), symlink_location, target_is_directory=True) # The structure of the test directory is now: # . diff --git a/testing/conftest.py b/testing/conftest.py index 90cdcb869fd..5ce0c0183b7 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -1,3 +1,4 @@ +import os import re import sys from typing import List @@ -136,6 +137,24 @@ def testdir(testdir: Testdir) -> Testdir: return testdir +@pytest.fixture +def symlink_or_skip(): + """Return a function to create symlinks or skip the test. + + On Windows `os.symlink` is available, but normal users require special + admin privileges to create symlinks. + """ + + def wrap_os_symlink(src, dst, *args, **kwargs): + try: + os.symlink(src, dst, *args, **kwargs) + except OSError as e: + pytest.skip("os.symlink({!r}) failed: {!r}".format((src, dst), e)) + assert os.path.islink(dst) + + return wrap_os_symlink + + @pytest.fixture(scope="session") def color_mapping(): """Returns a utility class which can replace keys in strings in the form "{NAME}" From 2d28bb998bd2a6ee7c2650952446ae809b1bf938 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 02:00:38 +0100 Subject: [PATCH 6/8] Cover non-symlink support also, restore behavior before e8c220b9b --- testing/acceptance_test.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 46f88ab6462..96e7dab592f 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -786,7 +786,11 @@ def test_cmdline_python_package_symlink( d_local = testdir.mkdir("local") symlink_location = os.path.join(str(d_local), "lib") - symlink_or_skip(str(d), symlink_location, target_is_directory=True) + try: + symlink_or_skip(str(d), symlink_location, target_is_directory=True) + has_symlinks = True + except pytest.skip.Exception: + has_symlinks = False # The structure of the test directory is now: # . @@ -808,16 +812,24 @@ def test_cmdline_python_package_symlink( # module picked up in symlink-ed directory: # It picks up local/lib/foo/bar (symlink) via sys.path. - result = testdir.runpytest("--pyargs", "-v", "foo.bar") - testdir.chdir() + result = testdir.runpytest("--pyargs", "-vv", "foo.bar") assert result.ret == 0 - result.stdout.fnmatch_lines( - [ - "lib/foo/bar/test_bar.py::test_bar PASSED*", - "lib/foo/bar/test_bar.py::test_other PASSED*", - "*2 passed*", - ] - ) + if has_symlinks: + result.stdout.fnmatch_lines( + [ + "lib/foo/bar/test_bar.py::test_bar <- local/lib/foo/bar/test_bar.py PASSED*", + "lib/foo/bar/test_bar.py::test_other <- local/lib/foo/bar/test_bar.py PASSED*", + "*2 passed*", + ] + ) + else: + result.stdout.fnmatch_lines( + [ + "*lib/foo/bar/test_bar.py::test_bar PASSED*", + "*lib/foo/bar/test_bar.py::test_other PASSED*", + "*2 passed*", + ] + ) def test_cmdline_python_package_not_exists(self, testdir): result = testdir.runpytest("--pyargs", "tpkgwhatv") From 33340a809cfa7ad0776cd5cfe4384a5ed60efac6 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 02:15:38 +0100 Subject: [PATCH 7/8] fixup! Cover non-symlink support also, restore behavior before e8c220b9b --- testing/acceptance_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 96e7dab592f..fef057a1c6e 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -825,8 +825,8 @@ def test_cmdline_python_package_symlink( else: result.stdout.fnmatch_lines( [ - "*lib/foo/bar/test_bar.py::test_bar PASSED*", - "*lib/foo/bar/test_bar.py::test_other PASSED*", + "local/lib/foo/bar/test_bar.py::test_bar PASSED*", + "local/lib/foo/bar/test_bar.py::test_other PASSED*", "*2 passed*", ] ) From 0499b13f7ca29defefae01f371cd38cadae1a43e Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 14 Feb 2020 02:15:49 +0100 Subject: [PATCH 8/8] doc --- testing/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/conftest.py b/testing/conftest.py index 5ce0c0183b7..02e095cb2c9 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -139,7 +139,7 @@ def testdir(testdir: Testdir) -> Testdir: @pytest.fixture def symlink_or_skip(): - """Return a function to create symlinks or skip the test. + """Return a function that creates a symlink or raises ``Skip``. On Windows `os.symlink` is available, but normal users require special admin privileges to create symlinks.