poetry: Some tests fail because of wrong rights on /foo

Not recommending that. What I am saying is you need to run your tests in a virtual environment. We do not really support running the test suite under the system interpretor.

From the package maintainers’ point of view running the test suite using the system interpreter (and not in the virtual environment) is exactly what you want to do. This way you can test whether the software behaves correctly under the exact Python version that is currently packaged with all needed dependencies (that often have some downstream patches).

Eli by using arguments like --system-site-packages and --without-pip is effectively trying to create a virtual environment that behaves like there was none.

Why is this bug report closed just because some unrelated bug mentioned 6 comments in happens to be resolved?

The error seems to have moved around a bit, but I still cannot package poetry due to:

__________________ test_builder_should_execute_build_scripts ___________________

extended_without_setup_poetry = <poetry.poetry.Poetry object at 0x7fa15ce21790>

    def test_builder_should_execute_build_scripts(extended_without_setup_poetry):
        env = MockEnv(path=Path("/foo"))
        builder = EditableBuilder(extended_without_setup_poetry, env, NullIO())
    
>       builder.build()

tests/masonry/builders/test_editable_builder.py:214: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
poetry/masonry/builders/editable.py:57: in build
    added_files += self._add_scripts()
poetry/masonry/builders/editable.py:150: in _add_scripts
    with script_file.open("w", encoding="utf-8") as f:
/usr/lib/python3.8/pathlib.py:1221: in open
    return io.open(self, mode, buffering, encoding, errors, newline,
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = PosixPath('/usr/bin/foo'), name = '/usr/bin/foo', flags = 524865
mode = 438

    def _opener(self, name, flags, mode=0o666):
        # A stub for the opener argument to built-in open()
>       return self._accessor.open(self, flags, mode)
E       PermissionError: [Errno 13] Permission denied: '/usr/bin/foo'

/usr/lib/python3.8/pathlib.py:1077: PermissionError

Here is an example where #3255 and #3107 (added some additional fixes to support these cases) are applied, without a virtualenvironment and test run by an unprivileged user.

Using alpine as the example since I am guessing that is the one with build failures still based on above comments.

podman run --rm -i --entrypoint sh python:3.8-alpine <<EOF
set -xe
apk --quiet add build-base libffi-dev openssl-dev git
pip install -q 'pytest>=5.4.3,<5.5' 'pytest-mock>=1.9,<2' pytest-cov pytest-sugar httpretty
cd /opt
git clone https://github.com/python-poetry/poetry.git
cd poetry
git fetch origin pull/3255/head:3255
git checkout 3255
wget https://patch-diff.githubusercontent.com/raw/python-poetry/poetry/pull/3107.diff
git apply 3107.diff
pip install -q .
adduser -D developer
chown -R developer:developer /opt/poetry
su - developer
cd /opt/poetry
pytest tests/
EOF

From the package maintainers’ point of view running the test suite using the system interpreter (and not in the virtual environment) is exactly what you want to do. This way you can test whether the software behaves correctly under the exact Python version that is currently packaged with all needed dependencies (that often have some downstream patches).

The project test suite is not necessarily developed with distro packaging requirements in mind, but rather with the intent to test the project’s functional requirements. What this means is that certain assumptions about how it is run might have crept in that might not work within the a distro’s package build environment. The test suite, for example, tries to assess that the system interpreter (which to poetry is the interpretor that starts poetry) site can be modified and a script can be installed in it’s prefix (side-effect of another test case that installs a package as a pre-requisite). However, note that this is a functional test, and the failure is expected when the test suite is run with a user that does not have the privilege to modify the site - as will the real world scenario.

I am not suggesting that this cannot be improved. However, we do not, at this time, have the resources/bandwidth to ensure that the test suite itself works outside the current recommended development environment. We would definitely appreciate pull requests to make the situation better as these build environments are not readily accessible for us. Things might also get better with #3107 and #3255.

because the test suite is verifying that it can “install” to the site of the python interpretor that is running the test suite,

That’s the info we needed! 🎉

This works:

# a virtualenv is necessary gh#python-poetry/poetry#1645
virtualenv testenv
source testenv/bin/activate
# poetry as it will be packaged
PYTHONPATH="/home/abuild/rpmbuild/BUILDROOT/python-poetry-1.2.0~a0+pr3255-0.x86_64/usr/lib/python3.8/site-packages"
# use system site-packages, we can't get packages into the virtualenv by downloading
export PYTHONPATH+=":/usr/lib/python3.8/site-packages:/usr/lib64/python3.8/site-packages"
export PYTHONDONTWRITEBYTECODE=1
# pytest needs to be called from the virtualenv python interpreter
python -m pytest -v tests
deactivate

622 passed, 4 skipped, 1 warning in 68.44s (0:01:08)

______________________________________________________________ test_execute_executes_a_batch_of_operations ______________________________________________________________

config = <poetry.config.config.Config object at 0x7f06204f3640>, pool = <poetry.repositories.pool.Pool object at 0x7f0620596880>
io = <clikit.io.buffered_io.BufferedIO object at 0x7f0620599520>, tmp_dir = '/tmp/poetry_0fc5d49f', mock_file_downloads = None

    def test_execute_executes_a_batch_of_operations(
        config, pool, io, tmp_dir, mock_file_downloads
    ):
        config = Config()
        config.merge({"cache-dir": tmp_dir})
    
        env = MockEnv(path=Path(tmp_dir))
        executor = Executor(env, pool, config, io)
    
        file_package = Package(
            "demo",
            "0.1.0",
            source_type="file",
            source_url=Path(__file__)
            .parent.parent.joinpath(
                "fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl"
            )
            .resolve()
            .as_posix(),
        )
    
        directory_package = Package(
            "simple-project",
            "1.2.3",
            source_type="directory",
            source_url=Path(__file__)
            .parent.parent.joinpath("fixtures/simple_project")
            .resolve()
            .as_posix(),
        )
    
        git_package = Package(
            "demo",
            "0.1.0",
            source_type="git",
            source_reference="master",
            source_url="https://github.com/demo/demo.git",
        )
    
        ret = executor.execute(
            [
                Install(Package("pytest", "3.5.2")),
                Uninstall(Package("attrs", "17.4.0")),
                Update(Package("requests", "2.18.3"), Package("requests", "2.18.4")),
                Uninstall(Package("clikit", "0.2.3")).skip("Not currently installed"),
                Install(file_package),
                Install(directory_package),
                Install(git_package),
            ]
        )
    
    
        expected = """
    Package operations: 4 installs, 1 update, 1 removal
    
      • Installing pytest (3.5.2)
      • Removing attrs (17.4.0)
      • Updating requests (2.18.3 -> 2.18.4)
      • Installing demo (0.1.0 {})
      • Installing simple-project (1.2.3 {})
      • Installing demo (0.1.0 master)
    """.format(
            file_package.source_url, directory_package.source_url
        )
    
        expected = set(expected.splitlines())
        output = set(io.fetch_output().splitlines())
>       assert expected == output
E       assert {'',\n '  • Installing demo (0.1.0 '\n '/build/python-poetry/src/poetry-1.1.2/tests/fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl)',\n '  • Installing demo (0.1.0 master)',\n '  • Installing pytest (3.5.2)',\n '  • Installing simple-project (1.2.3 '\n '/build/python-poetry/src/poetry-1.1.2/tests/fixtures/simple_project)',\n '  • Removing attrs (17.4.0)',\n '  • Updating requests (2.18.3 -> 2.18.4)',\n 'Package operations: 4 installs, 1 update, 1 removal'} == {'',\n '      1073│         raise ValueError("I/O operation on closed path")',\n '      1074│ ',\n '      1075│     def _opener(self, name, flags, mode=0o666):',\n '      1076│         # A stub for the opener argument to built-in open()',\n '      1078│ ',\n '      1079│     def _raw_open(self, flags, mode=0o777):',\n '      1080│         ',\n '      1081│         Open the file pointed by this path and return a file '\n 'descriptor,',\n '    → 1077│         return self._accessor.open(self, flags, mode)',\n '  PermissionError',\n "  [Errno 13] Permission denied: '/usr/bin/baz'",\n '  at /usr/lib/python3.8/pathlib.py:1077 in _opener',\n '  • Installing demo (0.1.0 '\n '/build/python-poetry/src/poetry-1.1.2/tests/fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl)',\n '  • Installing demo (0.1.0 master)',\n '  • Installing pytest (3.5.2)',\n '  • Installing simple-project (1.2.3 '\n '/build/python-poetry/src/poetry-1.1.2/tests/fixtures/simple_project)',\n '  • Removing attrs (17.4.0)',\n '  • Updating requests (2.18.3 -> 2.18.4)',\n 'Package operations: 4 installs, 1 update, 1 removal'}
E         Extra items in the right set:
E         '  PermissionError'
E         '      1076│         # A stub for the opener argument to built-in open()'
E         '    → 1077│         return self._accessor.open(self, flags, mode)'
E         '  at /usr/lib/python3.8/pathlib.py:1077 in _opener'
E         '      1073│         raise ValueError("I/O operation on closed path")'
E         '      1079│     def _raw_open(self, flags, mode=0o777):'
E         '      1075│     def _opener(self, name, flags, mode=0o666):'
E         "  [Errno 13] Permission denied: '/usr/bin/baz'"
E         '      1080│         '
E         '      1074│ '
E         '      1078│ '
E         '      1081│         Open the file pointed by this path and return a file descriptor,'
E         Full diff:
E           {
E            '',
E         -  '      1073│         raise ValueError("I/O operation on closed path")',
E         -  '      1074│ ',
E         -  '      1075│     def _opener(self, name, flags, mode=0o666):',
E         -  '      1076│         # A stub for the opener argument to built-in open()',
E         -  '      1078│ ',
E         -  '      1079│     def _raw_open(self, flags, mode=0o777):',
E         -  '      1080│         ',
E         -  '      1081│         Open the file pointed by this path and return a file '
E         -  'descriptor,',
E         -  '    → 1077│         return self._accessor.open(self, flags, mode)',
E         -  '  PermissionError',
E         -  "  [Errno 13] Permission denied: '/usr/bin/baz'",
E         -  '  at /usr/lib/python3.8/pathlib.py:1077 in _opener',
E            '  • Installing demo (0.1.0 '
E            '/build/python-poetry/src/poetry-1.1.2/tests/fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl)',
E            '  • Installing demo (0.1.0 master)',
E            '  • Installing pytest (3.5.2)',
E            '  • Installing simple-project (1.2.3 '
E            '/build/python-poetry/src/poetry-1.1.2/tests/fixtures/simple_project)',
E            '  • Removing attrs (17.4.0)',
E            '  • Updating requests (2.18.3 -> 2.18.4)',
E            'Package operations: 4 installs, 1 update, 1 removal',
E           }

tests/installation/test_executor.py:126: AssertionError

I discovered the cause of this error when modifying this: https://github.com/python-poetry/poetry/blob/68d6939f2e9cac1178fc5ffbcc3bac7a9db43fce/tests/installation/test_executor.py#L98-L126

so the assert 0 == executor.execute no longer masked the output which is tested by: assert expected == output

I have added some cleanup for the tests suites so that they work better for the use cases being described above. I am assuming (since it is not entirely clear), that the environments are your typical distro packaging environments. #3255

@bnavigator this should allow you to re-enable most of those tests. Please let me know if additional tests are still failining. Particularly, if any tests still require pypi access or if data is written to non temporary directories.

Regarding the use of dephell, I am quite curious as to why this is being used for building a PEP 517 package. Sounds like it introduces more problems than what it solves.

I’d also urge that the coversation be kept constructive.