Skip to content

Azure Pipelines: Test on Python 3.8 #6159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 27, 2019
Merged

Azure Pipelines: Test on Python 3.8 #6159

merged 1 commit into from
Dec 27, 2019

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Nov 9, 2019

  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features, improvements, and removals/deprecations.
  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Add yourself to AUTHORS in alphabetical order.

Python 3.8 has just been added to Azure Pipelines

Does this go in master or features? I guess it's an improvement, so features.

Does this need a changelog file? I guess not, but let me know if otherwise.

@hugovk hugovk added platform: windows windows platform-specific problem type: infrastructure improvement to development/releases/CI structure labels Nov 9, 2019
@hugovk
Copy link
Member Author

hugovk commented Nov 9, 2019

Python 3.8 is failing on Windows:

================================== FAILURES ===================================
_________ TestInvocationVariants.test_cmdline_python_package_symlink __________
[gw0] win32 -- Python 3.8.0 d:\a\1\s\.tox\py38-xdist\scripts\python.exe

self = <acceptance_test.TestInvocationVariants object at 0x0000019E9C203DC0>
testdir = <Testdir local('C:\\Users\\VssAdministrator\\AppData\\Local\\Temp\\pytest-of-VssAdministrator\\pytest-0\\popen-gw0\\test_cmdline_python_package_symlink0')>
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x0000019E9C2FBB50>

    @pytest.mark.skipif(not hasattr(os, "symlink"), reason="requires symlinks")
    def test_cmdline_python_package_symlink(self, testdir, monkeypatch):
        """
        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"
        d = testdir.mkdir(dirname)
        foo = d.mkdir("foo")
        foo.ensure("__init__.py")
        lib = foo.mkdir("bar")
        lib.ensure("__init__.py")
        lib.join("test_bar.py").write(
            "def test_bar(): pass\ndef test_other(a_fixture):pass"
        )
        lib.join("conftest.py").write(
            "import pytest\[email protected]\ndef a_fixture():pass"
        )
    
        d_local = testdir.mkdir("local")
        symlink_location = os.path.join(str(d_local), "lib")
        os.symlink(str(d), symlink_location, target_is_directory=True)
    
        # The structure of the test directory is now:
        # .
        # \u251c\u2500\u2500 local
        # \u2502   \u2514\u2500\u2500 lib -> ../lib
        # \u2514\u2500\u2500 lib
        #     \u2514\u2500\u2500 foo
        #         \u251c\u2500\u2500 __init__.py
        #         \u2514\u2500\u2500 bar
        #             \u251c\u2500\u2500 __init__.py
        #             \u251c\u2500\u2500 conftest.py
        #             \u2514\u2500\u2500 test_bar.py
    
        # NOTE: the different/reversed ordering is intentional here.
        search_path = ["lib", os.path.join("local", "lib")]
        monkeypatch.setenv("PYTHONPATH", prepend_pythonpath(*search_path))
        for p in search_path:
            monkeypatch.syspath_prepend(p)
    
        # 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()
>       assert result.ret == 0
E       assert <ExitCode.INTERRUPTED: 2> == 0
E         -<ExitCode.INTERRUPTED: 2>
E         +0

D:\a\1\s\testing\acceptance_test.py:820: AssertionError
---------------------------- Captured stdout call -----------------------------
============================= test session starts =============================
platform win32 -- Python 3.8.0, pytest-5.2.3.dev219+ga819d0cc8, py-1.8.0, pluggy-0.13.0 -- d:\a\1\s\.tox\py38-xdist\scripts\python.exe
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('D:\\a\\1\\s\\.hypothesis\\examples')
rootdir: C:\Users\VssAdministrator\AppData\Local\Temp\pytest-of-VssAdministrator\pytest-0\popen-gw0\test_cmdline_python_package_symlink0
plugins: hypothesis-4.43.8, forked-1.1.3, xdist-1.30.0
collecting ... collected 0 items / 1 error

=================================== ERRORS ====================================
________________________ ERROR collecting test session ________________________
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\config\__init__.py:459: in _importconftest
    return self._conftestpath2mod[key]
E   KeyError: WindowsPath('C:/Users/VssAdministrator/AppData/Local/Temp/pytest-of-VssAdministrator/pytest-0/popen-gw0/test_cmdline_python_package_symlink0/lib/foo/bar/conftest.py')

During handling of the above exception, another exception occurred:
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\config\__init__.py:465: in _importconftest
    mod = conftestpath.pyimport()
d:\a\1\s\.tox\py38-xdist\lib\site-packages\py\_path\local.py:721: in pyimport
    raise self.ImportMismatchError(modname, modfile, self)
E   py._path.local.LocalPath.ImportMismatchError: ('foo.bar.conftest', 'local\\lib\\foo\\bar\\conftest.py', local('C:\\Users\\VssAdministrator\\AppData\\Local\\Temp\\pytest-of-VssAdministrator\\pytest-0\\popen-gw0\\test_cmdline_python_package_symlink0\\lib\\foo\\bar\\conftest.py'))

During handling of the above exception, another exception occurred:
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\runner.py:229: in from_call
    result = func()
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\runner.py:249: in <lambda>
    call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\main.py:494: in collect
    yield from self._collect(initialpart)
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\main.py:522: in _collect
    col = self._collectfile(pkginit, handle_dupes=False)
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\main.py:591: in _collectfile
    ihook = self.gethookproxy(path)
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\main.py:435: in gethookproxy
    my_conftestmodules = pm._getconftestmodules(fspath)
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\config\__init__.py:437: in _getconftestmodules
    mod = self._importconftest(conftestpath)
d:\a\1\s\.tox\py38-xdist\lib\site-packages\_pytest\config\__init__.py:473: in _importconftest
    raise ConftestImportFailure(conftestpath, sys.exc_info())
E   _pytest.config.ConftestImportFailure: (local('C:\\Users\\VssAdministrator\\AppData\\Local\\Temp\\pytest-of-VssAdministrator\\pytest-0\\popen-gw0\\test_cmdline_python_package_symlink0\\lib\\foo\\bar\\conftest.py'), (<class 'py._path.local.LocalPath.ImportMismatchError'>, ImportMismatchError('foo.bar.conftest', 'local\\lib\\foo\\bar\\conftest.py', local('C:\\Users\\VssAdministrator\\AppData\\Local\\Temp\\pytest-of-VssAdministrator\\pytest-0\\popen-gw0\\test_cmdline_python_package_symlink0\\lib\\foo\\bar\\conftest.py')), <traceback object at 0x0000019E9C370F00>))
!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
============================== 1 error in 0.30s ===============================

https://pytest-dev.visualstudio.com/pytest/_build/results?buildId=2899

Any ideas why?

@blueyed

This comment has been minimized.

@hugovk

This comment has been minimized.

@nicoddemus
Copy link
Member

nicoddemus commented Nov 12, 2019

Oh sorry, confused this error with something else.

@blueyed
Copy link
Contributor

blueyed commented Nov 25, 2019

    monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False)

Reminded me of 1abb08d.
Edit: gets deleted there though.. giving it a try anyway.

@blueyed
Copy link
Contributor

blueyed commented Nov 25, 2019

@hugovk
Last try from my side using PY_IGNORE_IMPORTMISMATCH=1.
It looks though like symlink handling might have changed with py38 on Windows in general. Certainly easier with somebody having Windows available to debug this locally.

@hugovk
Copy link
Member Author

hugovk commented Nov 26, 2019

Azure Pipelines is now green, thank you for following up!

@blueyed
Copy link
Contributor

blueyed commented Nov 26, 2019

@hugovk
Great. Do you have access to Windows locally yourself to investigate? I think PY_IGNORE_IMPORTMISMATCH=1 either just hides a real problem or shows there's an issue with pylib in this regard. Maybe also a cpython bug.

@hugovk
Copy link
Member Author

hugovk commented Nov 26, 2019

No, unfortunately I don't have access to a Windows machine.

@nicoddemus
Copy link
Member

nicoddemus commented Dec 26, 2019

I found the underlying cause, py.path.local.samefile doesn't work with symlinks in Python 3 on Windows (pytest-dev/py#229).

This is only apparent in Python 3.8 on Windows, and the overall picture is complicated:

  1. Pythons <3.8 don't have a real implementation of os.path.realpath:

    # realpath is a no-op on systems without islink support
    realpath = abspath

    https://github.com/python/cpython/blob/43364a7ae01fbe4288ef42622259a0038ce1edcc/Lib/ntpath.py#L530-L531

    While Python 3.8+ does:

        def realpath(path):
            path = normpath(path)
            if isinstance(path, bytes):
                prefix = b'\\\\?\\'
                unc_prefix = b'\\\\?\\UNC\\'
                new_unc_prefix = b'\\\\'
                cwd = os.getcwdb()
                # bpo-38081: Special case for realpath(b'nul')
                if normcase(path) == normcase(os.fsencode(devnull)):
                    return b'\\\\.\\NUL'

    https://github.com/python/cpython/blob/fa919fdf2583bdfead1df00e842f24f30b2a34bf/Lib/ntpath.py#L600-L638

  2. py.path.local.realpath forwards its work to os.path.realpath:

        def realpath(self):
            """ return a new path which contains no symbolic links."""
            return self.__class__(os.path.realpath(self.strpath))

    https://github.com/pytest-dev/py/blob/1e99d20f31ef511fc2f1a9e49ba970acad441d4e/py/_path/local.py#L594-L596

  3. When listing conftest.py files, we rely on realpath to actually resolve symlinks for us:

    for parent in directory.realpath().parts():
    if self._confcutdir and self._confcutdir.relto(parent):
    continue
    conftestpath = parent.join("conftest.py")
    if conftestpath.isfile():
    mod = self._importconftest(conftestpath)
    clist.append(mod)

  4. samefile is used in pyimport() to check for a mismatch:

    https://github.com/pytest-dev/py/blob/1e99d20f31ef511fc2f1a9e49ba970acad441d4e/py/_path/local.py#L715

And here is the problem: on Pythons <3.8, realpath() and samefile() don't support symlinks at all, and because of that, realpath().parts() returns paths with symbolic links, which samefile() assumes are equal because it is comparing just
the paths.

On Python 3.8 though, realpath does resolve symlinks, and because samefile hard codes that it doesn't work on WIN32 (see pytest-dev/py#229), the checks now fail.

Fixing py.path.local.samefile makes it possible to fix this test (I've checked locally).

Phew! 😓

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Dec 26, 2019
As discussed in pytest-dev#6159,
we are waiting for a new py release (pytest-dev/py#229)
@nicoddemus nicoddemus changed the base branch from features to master December 27, 2019 16:09
@nicoddemus
Copy link
Member

Force pushed cleaning up the other commits and targeting master after py-1.8.1 has been released. 🤞

@nicoddemus
Copy link
Member

🎉

@nicoddemus nicoddemus merged commit afa899d into pytest-dev:master Dec 27, 2019
@hugovk hugovk deleted the features-azurepipelines-3.8 branch December 27, 2019 17:28
@hugovk
Copy link
Member Author

hugovk commented Dec 27, 2019

Good work getting to the bottom of this, and thanks for finishing it up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: windows windows platform-specific problem type: infrastructure improvement to development/releases/CI structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants