Some tests fail because of wrong rights on /foo #1645
Comments
|
Hello @mcepl , can you please add some more context to your issue? What are you trying to do? What is your expected behavior? Thanks a lot. fin swimmer |
|
Trying to build package of poetry for openSUSE and trying to run the test suite (as with any other Python package). There is complete build log with all information to the last really excruciating details attached to the previous comment. If I may add one question not specifically related to this issue (unfortunately, I haven’t found any IRC, Gitter, matrix, etc. channel, email list related to poetry; I tried IRC channel Obviously |
|
Arch Linux now packages poetry by generating the setup.py for poetry and all its dependencies using https://github.com/dephell/dephell @jayvdb has packaged dephell for OpenSUSE if I remember correctly. |
@mcepl: There is a discordapp server. See here: #969 (comment) |
|
I almost got all tests working without a venv using sed -i 's:"/foo":"/tmp/foo":' tests/conftest.py
sed -i 's:/foo:/tmp/foo:' tests/console/commands/test_config.py tests/config/test_config.py
sed -i 's:/foo/virtualenv:/tmp/foo/virtualenv:' tests/utils/test_env.py
mkdir ~/.bin
ln -s /usr/bin/python3 ~/.bin/python
export PATH=$PATH:~/.binThere were still some failures, but only a few. |
|
@eli-schwartz , ya, correct, and I have been doing the poetry packaging for a while now. Thanks for raising dephell/dephell#330 about the current annoyance with that approach, but it is minor and easy to workaround. Creating a testenv works around the /foo test failures, obviously. I still have one failure, |
|
@jayvdb I've ignored that test by running the testsuite using: Because the unittest in question is exercising poetry's ability to autogenerate exclusions based on a .gitignore, so it assumes that it's being run from a git checkout of poetry in order to successfully mock |
|
Thanks. I did the same without knowing precisely the problem. It looked more like a test logic failure than runtime code problem. Happy to have someone one step ahead of me ;-) |
For reference: This is no longer necessary with 1.1.1. The test moved to poetry-core. |
|
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: |
This comment has been minimized.
This comment has been minimized.
I am a bit lost: do we have opened ticket for this here? |
No we just referenced this one here. Bad practise on our side, nothing to chastice the python-poetry developers |
|
There are a bunch of errors cropping up in various places, but they all have to do with the exact same problem -- the testsuite keeps on trying to do work, in the directory After modifying the testsuite to properly report errors in some deeply nested thing, I found the cause of another It seems reasonable that they're all the same underlying issue, represented by this ticket. EDIT: posted the resulting log below, it was |
- ensure tests rely on temporary cache directory - remove external http call requirement for lock --no-update Relates-to: python-poetry#1645
|
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. |
Thanks. Will test and report back.
+1 from me. dephell brings its own dependency hell (dephell/dephell#347) and obviously fails too often on reading compliant specification files. Historically, it was necessary to create a setup.py and use the standard setuptools way of installing. Now we can also use
+1 |
- ensure tests rely on temporary cache directory - remove external http call requirement for lock --no-update Relates-to: python-poetry#1645
|
Meh, #3255 does not apply cleanly onto 1.1.3 and after rebasing, two tests still fail: poetry-pr3255-testcleanup-1.1.3.patch.txt Edit: I see you force pushed the PR after my first rebase. Same result with second rebase |
|
BTW, the venv is still necessary because many tests call |
|
Directly building from 43cd07c has the same effect. |
The use of "python" is intentional in most cases as commands need to be executed by a different interpretor than that which poetry is running under. As for the failing test, I have pushed a fix for the |
Do you mean a different version or just a different process? If the latter, you still should use |
|
With e1ba4f8 the test still fails, because you still want to write into the system root: |
I discovered the cause of this error when modifying this: poetry/tests/installation/test_executor.py Lines 98 to 126 in 68d6939 so the |
|
@bnavigator from your logs it seems that your are running as an unprivileged user, with system interpretor. For that scenario, there are a few bugs in
Different version when dealing with virtual environments. When dealing with system environment, we still use This works for me without a virtual environment. FROM python:3.8
RUN curl -sL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python -
COPY . /opt/poetry
WORKDIR /opt/poetry
RUN ~/.poetry/bin/poetry config virtualenvs.create false
RUN ~/.poetry/bin/poetry install
RUN pytest tests/ |
|
Yeah, that was what I did -- saved it as |
|
Running tests with system privileges is no option. Neither for rpmbuild (openSUSE, Fedora, ...) nor for Eli's Archlinux. |
|
@bnavigator that fix (#3107) is not in master yet.
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. I have updated the PR again with a few more fixes. |
|
Question: does poetry's CI test runner get run as root, or does it get run via a virtualenv as a non-privileged user as you just suggested? |
|
poetry/.github/workflows/main.yml Lines 62 to 68 in 68d6939 Poetry implicitly manages the virtual environment. |
|
But, does it actually test if the suite can run successfully as an unprivileged user? Or does it only work because e.g. github workflows happen to run as root? |
|
@eli-schwartz the workflow does not explcitly drop down to an unprivileged user, but that is out of scope for the project. Most developers do not run the tests suites as the root user anyway. Note that the issue that you are facing is effectively because the test suite is verifying that it can "install" to the |
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 |
|
OK, tried to run tests like this: I'm down to fewer failures, but it still seems to be trying to create |
|
Did you forget to activate? |
|
Why would I need to activate, a function that simply adds the venv python executable to the $PATH, if I explicitly invoke |
|
Note: I've gotten this in 1.1.2 and 1.1.3, but I did not try any betas or git master... |
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 |
|
I don't know, maybe because the quirky poetry test suite internals still call the python from the PATH? |
|
Well, I tried rerunning the test with: (
source ./poetrytests/bin/activate
python -m pytest
)And it had no effect. 1.1.3 still fails in the same place. |
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. |
It sounds like #3107 would be a critically important improvement. |
Honestly, I do not think we can help you here because this is a build environment issue. Our CI works within virtual environments and will work just fine, and even development environments that run as unprivileged users work just fine both under poetry managed venv and tox managed venvs. Here is a setup with an unprivileged user using system interpreter under alpine that works, maybe it will help. podman run --rm -i --entrypoint sh python:3.8-alpine <<EOF
set -xe
apk --quiet add build-base libffi-dev openssl-dev git # dependencies requried for cryptography
pip install -q poetry # install poetry to system
adduser -D developer
su - developer
set -xe
git clone https://github.com/python-poetry/poetry.git
cd poetry
poetry config virtualenvs.in-project true
which poetry
poetry install # use system poetry to create venv and install required dependencies
source .venv/bin/activate # manual activation to demonstrate
which python
pytest -q tests/
EOF
The
The change proposed in #3107 is to add |
|
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 |
|
FYI:
Here is the scenario for
So if a project decides to require full poetry for end-user installation, it would be great for poetry to support
PIP as PEP517 frontend with poetry-core backend can deal with this using the "build wheel and install it into BUILDROOT approach": poetry-core-obs_log.txt The I know that this defeats the intention of poetry. |
|
@bnavigatorI think with the above patches your build should work without internet connectivity as well as without a virtual environment.
I do not understand this bit. If I understand correct, if I feel that there is some misunderstanding here on what poetry is intended for. For cases where you are building a package for an application or library, I believe you must follow PEP 517 build by using a PEP 517 frontend. The only reason you will ever need
Already possible with the above PRs.
This is something that will arrive with bundling (but not exactly as you expect it to). That said you shouldn't have to use it because
Can't you just not use the |
As poetry currently builds, installs and tests fine (with #1645 (comment)), I see no reason to merge the patches before you make a new release.
Only the
Exactly. So any project deploying their sdist package to PyPI should never depend on |
- ensure tests rely on temporary cache directory - remove external http call requirement for lock --no-update Relates-to: #1645
It is now in the release of 1.1.4, right? Unfortunately, the 1.1.4 tagged source code continues to fail the test mentioned above:
The only change is in the random identifier, which is now |
|
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I am on the latest Poetry version.
I have searched the issues of this repo and believe that this is not a duplicate.
If an exception occurs when executing a command, I executed it again in debug mode (
-vvvoption).OS version and name: openSUSE/Tumbleweed (Linux)
Poetry version: 1.0.0b8
Issue
Plenty of tests fail because of the traceback similar to this one (from ):
Complete build log
The text was updated successfully, but these errors were encountered: