Skip to content
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

Fix failing CI tests #292

Closed
apyrgio opened this issue Dec 13, 2022 · 4 comments · Fixed by #293
Closed

Fix failing CI tests #292

apyrgio opened this issue Dec 13, 2022 · 4 comments · Fixed by #293
Assignees
Labels
bug Something isn't working
Milestone

Comments

@apyrgio
Copy link
Contributor

apyrgio commented Dec 13, 2022

Our CI tests have recently started to fail on their own, due to upstream changes. See an example CI run here.

We see 3 tests that are failing:

  1. run-lint: The poetry install command fails with exit code 1, and no log message.

  2. build-debian-bookworm: The "Install dependencies (deb)" task fails with Unable to locate package python-all.

  3. convert-test-docs: The "Install poetry dependencies" step fails with:

    ERROR: launchpadlib 1.10.13 requires testresources, which is not installed.
    ERROR: importlib-resources 5.10.1 has requirement zipp>=3.1.0; python_version < "3.10", but you'll have zipp 1.0.0 which is incompatible.
    
@apyrgio apyrgio added the bug Something isn't working label Dec 13, 2022
@apyrgio apyrgio self-assigned this Dec 13, 2022
@apyrgio apyrgio added this to the 0.4.1 milestone Dec 13, 2022
@apyrgio
Copy link
Contributor Author

apyrgio commented Dec 13, 2022

Fixing build-debian-bookworm

Problem

The python-all package refers to Python2 versions, which we don't support. Since Debian Bookworm is relatively bleeding edge, and Python 2 is EOL, it has completely dropped Python 2 packages, and therefore this package as well.

If we remove the python-all package from the list of the packages we install, then the Dangerzone package builds successfully in Debian Bookworm. However, it fails to build on Ubuntu Focal, because it somehow requires python-all. This may have to do with an stdeb issue from 2020: astraw/stdeb#153. This has been fixed in version 0.9.1, but Ubuntu Focal provides 0.8.5.

Solution

We can overcome this issue by installing python-all solely for Ubuntu Focal. We can add this as a separate step only for the build-ubuntu-focal task.

While working on this issue, it became obvious that we can do something about the extra packages we have to install in this step, and most probably are not required. Read the following section for more details.

Simplifying build-debian-bookworm

What we did is that we used a vanilla debian:bookworm container, mounted Dangerzone there, and attempted to run our ./install/linux/build-deb.py script there. Through trial and error, we found that the necessary packages for building the Dangerzone .deb are python3, python3-setuptools, python3-stdeb, and dh-python.

The rest of the packages are necessary if we run the unit tests with dh_auto_test, which is the default behavior. This behavior can be disabled though with the DEB_BUILD_OPTIONS=nocheck envvar. See also:

By disabling tests while building the package, we don't sacrifice much, since unittest never actually ran the Dangerzone tests, as they are PyTest test cases.

@apyrgio
Copy link
Contributor Author

apyrgio commented Dec 14, 2022

Fixing run-lint

I tried to reproduce the error locally, in a debian:bullseye container. Unfortunately, I didn't manage to, since the poetry install command always ran to completion. I tested if the issue manifests when the standard streams are not TTYs, and the command still succeeded.

Possible causes for this issue may be:

  1. Changes to the CircleCI runners
  2. Changes to the Poetry package on PyPI

In both cases, we would need to continue the debugging in a CircleCI runner. There is a way to avoid all this hassle though, and even improve this step:

  1. Change the base image for this step from debian:bullseye to debian:bookworm.
  2. Install python3-poetry from the official repos, instead of downloading it from PyPI. This has the benefit of using more stable versions of Poetry than the latest ones in PyPI.
    • By default, installing python3-poetry brings lots of packages with it, some of them GUI related. We can skip those with --no-install-recommends.
  3. Because Debian Bookworm provides python-poetry==1.2.2, we can use the --only dev flag, and install only the dev dependencies.

@apyrgio
Copy link
Contributor Author

apyrgio commented Dec 14, 2022

Fixing convert-test-docs

Turns out that this step does not fail with this error:

ERROR: launchpadlib 1.10.13 requires testresources, which is not installed.
ERROR: importlib-resources 5.10.1 has requirement zipp>=3.1.0; python_version < "3.10", but you'll have zipp 1.0.0 which is incompatible.

Instead, it fails for the same reason that run-lint fails, i.e., due to a Poetry 1.3 bug: python-poetry/poetry#7184. A simple workaround for now is to pin Poetry to 1.2.2, and then remove the pin once it's fixed upstream, as it affects only installations from PyPI.

Note that we also tested replacing machine runner ubuntu-2004 with ubuntu-2204, and install Poetry from the official repos. Turns out that they provide only Poetry 1.1, but we need Poetry 1.2, which supports the group syntax.

@deeplow
Copy link
Contributor

deeplow commented Dec 14, 2022

Fixing debian-bookworm

With the above changes, we greatly simplify the build dependencies, and shave off roughly half a minute from each build step.

Thanks for the careful analysis. I am inclined to agree with this assessment. Minimizing the dependencies will also decrease the chance of dependency issues in the future.

Fixing run-lint

In both cases, we would need to continue the debugging in a CircleCI runner. There is a way to avoid all this hassle though, and even improve this step:

Great! Since this is only linting, it shouldn't matter that much which version we're running.

apyrgio added a commit that referenced this issue Dec 14, 2022
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)
apyrgio added a commit that referenced this issue Dec 14, 2022
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. Doing so
has two issues:

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]: astraw/stdeb#153
[2]: #292 (comment)
apyrgio added a commit that referenced this issue Dec 14, 2022
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 added a commit that referenced this issue Dec 15, 2022
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)
apyrgio added a commit that referenced this issue Dec 15, 2022
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]: astraw/stdeb#153
[2]: #292 (comment)
apyrgio added a commit that referenced this issue Jan 25, 2023
Temporarily skip building Debian Bookworm packages, due to an upstream
deprecation of `python-all` package [1]. The proper fix is in the `main`
branch, but we don't need to cherry-pick this in this hotfix branch.

[1]: #292 (comment)
apyrgio added a commit that referenced this issue Jan 25, 2023
Temporarily skip building Debian Bookworm packages, due to an upstream
deprecation of `python-all` package [1]. The proper fix is in the `main`
branch, but we don't need to cherry-pick this in this hotfix branch.

[1]: #292 (comment)
apyrgio added a commit that referenced this issue Feb 6, 2023
Remove a Poetry version pin to 1.2.2, which causes installation issues
on systems with Python 3.11.

The pin was originally introduced because Poetry 1.3 was deemed
unstable, due to the following bugs:

* #292 (comment)
* https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029156

The first problem still stands, but we can circumvent it with the
`--no-ansi` flag, at no functionality cost. The second problem has been
resolved, but it never affected Ubuntu Focal in the first place.

Refs #292
apyrgio added a commit that referenced this issue Feb 7, 2023
Remove a Poetry version pin to 1.2.2, which causes installation issues
on systems with Python 3.11.

The pin was originally introduced because Poetry 1.3 was deemed
unstable, due to the following bugs:

* #292 (comment)
* https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029156

The first problem still stands, but we can circumvent it with the
`--no-ansi` flag, at no functionality cost. The second problem has been
resolved, but it never affected Ubuntu Focal in the first place.

Refs #292
apyrgio added a commit that referenced this issue Feb 7, 2023
Remove a Poetry version pin to 1.2.2, which causes installation issues
on systems with Python 3.11.

The pin was originally introduced because Poetry 1.3 was deemed
unstable, due to the following bugs:

* #292 (comment)
* https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029156

The first problem still stands, but we can circumvent it with the
`--no-ansi` flag, at no functionality cost. The second problem has been
resolved, but it never affected Ubuntu Focal in the first place.

Refs #292
apyrgio added a commit that referenced this issue Feb 7, 2023
Remove a Poetry version pin to 1.2.2, which causes installation issues
on systems with Python 3.11.

The pin was originally introduced because Poetry 1.3 was deemed
unstable, due to the following bugs:

* #292 (comment)
* https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029156

The first problem still stands, but we can circumvent it with the
`--no-ansi` flag, at no functionality cost. The second problem has been
resolved, but it never affected Ubuntu Focal in the first place.

Refs #292
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants