Fix failing CI tests #293
Fix failing CI tests #293
Conversation
deeplow
left a comment
deeplow
left a comment
There was a problem hiding this comment.
Thanks. Looks good overall. I have just added some minor comments.
| Suite: bionic | ||
| X-Python3-Version: >= 3.6 | ||
| X-Python3-Version: >= 3.6 | ||
| Setup-Env-Vars: DEB_BUILD_OPTIONS=nocheck |
There was a problem hiding this comment.
What do you think about adding a comment about the fact that this disables testing on package building? nocheck might not be an obvious indication of this. Of course on can always search online or check the commit message.
There was a problem hiding this comment.
Sure, I can add a message like:
# Do not trigger testing when we build the package. Assume that the user
# has tested the package already. For more info, see:
# https://github.com/freedomofpress/dangerzone/issues/292#issuecomment-1349967888
.circleci/config.yml
Outdated
| pip install poetry | ||
| poetry install # FIXME --dev-only once poetry 1.2.0 is out https://github.com/python-poetry/poetry/issues/2572 | ||
| apt-get install -y make python3 python3-poetry --no-install-recommends | ||
| poetry install --only dev --no-root |
There was a problem hiding this comment.
I see that a lot of dependencies are still installed this way -- beyond just the linting ones. This already solves the issue, but we can shorten the install time and bandwidth by moving lint dependencies into a group of their own in pyproject.toml:
For example, moving them into:
[tool.poetry.group.lint.dependencies]
black = "*"
isort = "*"
mypy = "*"
PySide2-stubs = "*"
Then we install with this line poetry install --only lint.
There was a problem hiding this comment.
Also, with the --only dev or --only lint, I don't think it needs --no-root since it only installs the group.
There was a problem hiding this comment.
Good thinking, I'll try it and report back.
There was a problem hiding this comment.
Works great, so I'm including a fix for it.
| aliases: | ||
| - &provide-podman | ||
| name: Provide Podman in Ubuntu Focal | ||
| command: ./install/linux/install-podman-ubuntu-focal.sh --repo-only |
There was a problem hiding this comment.
Since no longer needed, should we simplify the script by removing the --repo-only parameter?
There was a problem hiding this comment.
Given that it's an Ubuntu Focal script that we will drop soon, I wouldn't bother to be honest. Plus you never know when it will prove handy again :-/
|
Looks great. Feel free to merge. |
Create two separate groups for Poetry dependencies: 1. test: Dependencies required for testing Dangerzone. 2. lint: Dependencies required for linting the code with `make lint`.
Fix the failing run-lint test by switching to Debian Bookworm for this step, and installing Poetry 1.2.2 from the official repos. This way, we circumvent a bug [1] in Poetry 1.3 (released on PyPI) and we greatly simplify this step [2]. [1]: python-poetry/poetry#7184 [2]: #292 (comment)
Debian has removed the python-all package from its Bookworm repos, which breaks our CI tests. Looking into why python-all is required in the first place, we found that it's an artificial stdeb requirement [1], prior to 0.9.1 versions The only platform affected by this issue is Ubuntu Focal, so our solution is to install python-all specifically for that platform. Finally, we further simplify our build tasks [2] (on Debian-like distros) by not letting dh-python run tests when building the packages. Running the tests has some issues after all: 1. It requires installing all the runtime dependencies of Dangerzone, since it uses `python -m unittest discover` underneath. 2. It doesn't aid in the stability of the package, since unittest cannot run test cases for PyTest. [1]: stdeb/stdeb#153 [2]: #292 (comment)
Fix the failing convert-test-docs step, by pinning Poetry to version 1.2.2. This way, we avoid a bug in Poetry 1.3 [1], which was recently released on PyPI. [1]: python-poetry/poetry#7184 Closes #292
apyrgio
force-pushed
the
292-fix-tests
branch
from
faf4b53 to
fc313d8
Compare
Fix some failing CI tests, that started happening due to some upstream changes on Poetry and Debian Bookworm.
Closes #292