ci: run the test suite on pull requests and pushes to main - #190
Open
nelsonduarte wants to merge 2 commits into
Open
nelsonduarte wants to merge 2 commits into
nelsonduarte wants to merge 2 commits into
Conversation
Nothing ran pytest in CI. The suite only ever ran on developer machines, all of them Windows, so there was no evidence of how it behaves on a clean Linux machine. This adds .github/workflows/tests.yml, which runs the full suite on ubuntu-latest with Python 3.14 and QT_QPA_PLATFORM=offscreen, on every pull request against main, on every push to main, and on demand. The workflow is expected to be red on its first runs, on purpose. It is Ubuntu only and it does not block merges: main has no required status checks (confirmed through the branch protection API), and this change does not add any. Its value in this first pass is to surface what is broken on a clean Linux runner. Two failures are already known. Both are pre-existing, both are left in place, and neither should be silenced by weakening the assertion: * tests/test_updater_msix.py:198, in test_check_for_update_returns_release_for_nsis_when_newer. The _find_asset() assertion sits outside the `with patch.object(updater.sys, "platform", "win32")` block, so on Linux it looks up the Linux asset name, gets None, and raises TypeError. The defect is in the test; the production code is correct. * tests/test_round8_fixes.py::test_pdfapps_version_flag_runs. PyMuPDF 1.28.2 prints a deprecation notice about the legacy `fitz` API on stdout rather than stderr, so the output no longer starts with "PDFApps ". requirements.txt declares pymupdf>=1.28.0 with no upper bound, so a fresh install picks that release up. The test dependencies (pytest, pytest-qt, coverage) go into requirements-dev.txt, not requirements.txt. requirements.txt is the runtime surface that ships: it feeds the PyInstaller builds, it is the file security-deps.yml watches, and every entry in it must also be pinned for the Flatpak or explicitly omitted, as enforced by test_no_root_dependency_is_silently_dropped_from_the_flatpak. A test runner belongs in none of those. The header of requirements-dev.txt now also records why ruff, PyYAML and pytest-qt are mandatory for the suite rather than optional. Only libegl1 and libxkbcommon0 are installed from apt. Unlike the Linux leg of build.yml, libxcb-cursor0 is omitted on purpose: it is linked only by the xcb platform plugin, which the offscreen platform never loads. With the eight non-offscreen platform plugins physically deleted, the suite gave the same result as before (2 failed, 746 passed, 3 skipped). .gitignore gains .coverage, .coverage.* and htmlcov/, since coverage is now a declared dev dependency and leaves those behind in the repo root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The comment on "Install Qt system deps" in tests.yml pointed at build.yml by line number, said `ldd libqoffscreen.so` needs exactly libEGL, libxkbcommon and libxcb, and predicted a "could not load the Qt platform plugin" failure. Rewrite it against PySide6 6.11.2: libEGL.so.1 and libxkbcommon.so.0 are DT_NEEDED by libQt6Gui.so.6, so a missing one makes `from PySide6 import QtGui` raise ImportError, and libqoffscreen.so and libQt6Gui.so.6 also need libX11.so.6 and libGL.so.1. libxcb-cursor0 stays out because an offscreen run of the suite loads none of the four objects that need libxcb-cursor.so.0. The comment says "loaded", not "opened": Qt reads the metadata of the platform plugins from disk and only dlopens the one it selects. Under strace -f (openat, mmap, mprotect, munmap) and LD_DEBUG=files, a full offscreen run of the suite with PySide6 6.11.2, in ubuntu:24.04 and ubuntu:26.04 containers (Python 3.12.3 and 3.14.4), opened libqxcb.so once and mapped 19000 bytes of it PROT_READ, unmapped straight after; nothing mapped it PROT_EXEC, and ld.so loaded only libqoffscreen.so from the platforms directory. The package observations are anchored to the runner image (ubuntu-24.04 20260927.320.1) rather than to a run, whose log expires after 90 days. Evidence: run 36692428596, where apt-get installed libegl1 as a new package and found libxkbcommon0 already installed. Of libX11.so.6, libGL.so.1 and libxcb.so.1, only libGL.so.1 relies on the image alone: on clean Ubuntu 24.04 and 26.04 images, installing libegl1 also pulls in libx11-6 and libxcb1 via libegl-mesa0, but not libgl1. It is the one of the three to watch when ubuntu-latest moves to 26.04. Comment-only change: the parsed YAML is identical to the parent commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Deploying pdfapps with
|
| Latest commit: |
4b20cc8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://76b06158.pdfapps.pages.dev |
| Branch Preview URL: | https://ci-run-test-suite-on-pull-re.pdfapps.pages.dev |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
.github/workflows/tests.yml, the first workflow that runs the pytest suite in CI. Until now the suite only ran on developer machines, all of them Windows, so there was no evidence of how it behaves on a clean Linux machine.What the workflow does
ubuntu-latest, Python 3.14, withQT_QPA_PLATFORM=offscreen(mandatory: the suite instantiates real QWidgets, and the default xcb plugin aborts on a headless runner).main, every push tomain, andworkflow_dispatch.permissions: contents: read, actions pinned by SHA, 20-minute timeout, concurrency group per ref with cancel-in-progress.requirements.txtand thenrequirements-dev.txt, and runspython -m pytest -q -rs.What it does NOT do
mainhas no required status checks (confirmed through the branch protection API), and this PR does not add any.This workflow is red on purpose
The first runs are expected to fail. The value of this first pass is to find out what is broken on a clean Linux runner, not to show a green badge. Two failures are known, both pre-existing, and both are deliberately left in place. Please do not "fix" them by weakening or skipping the assertions; each needs its own change.
1.
tests/test_updater_msix.py:198In
test_check_for_update_returns_release_for_nsis_when_newer, the final assertionsits outside the
with patch.object(updater.sys, "platform", "win32")block. On Windows that goes unnoticed. On Linux,_find_asset()looks upPDFApps-Linux.tar.gz, getsNone, and the subscript raisesTypeError. The defect is in the test; the production code is correct.2.
tests/test_round8_fixes.py::test_pdfapps_version_flag_runsPyMuPDF 1.28.2 prints a deprecation notice about the legacy
fitzAPI on stdout (not stderr), so the output ofpython pdfapps.py --versionno longer starts withPDFApps.requirements.txtdeclarespymupdf>=1.28.0with no upper bound, so a fresh install picks that release up.Measured by QA before opening this PR, on Linux (Ubuntu, Python 3.14): 2 failed, 746 passed, 3 skipped. The first real CI run is the check that matters.
Why the test dependencies are in
requirements-dev.txtpytest,pytest-qtandcoveragego intorequirements-dev.txt, notrequirements.txt:security-deps.yml. That workflow's path filters arerequirements.txt,flatpak/requirements-pinned.txtand, on PRs only, the workflow file itself.requirements.txtmust also be pinned inflatpak/requirements-pinned.txtor listed inFLATPAK_OMITTED, as enforced bytests/test_flatpak_dependency_pins.py::test_no_root_dependency_is_silently_dropped_from_the_flatpak.requirements.txtis the shipped runtime surface. It feeds the PyInstaller builds inbuild.yml. A test runner has no business in the app bundle.The header of
requirements-dev.txtalso records whyruff,PyYAMLandpytest-qtare mandatory for the suite and not optional: without them some tests fail or error instead of skipping..gitignoregains.coverage,.coverage.*andhtmlcov/, sincecoverageis now a declared dev dependency.Why
libxcb-cursor0is not installedThe workflow installs only
libegl1andlibxkbcommon0. Diverging frombuild.yml:101, which also installslibxcb-cursor0, is intentional: that leg runs the real GUI app, while this one never leaves the offscreen platform.libxcb-cursor.so.0is linked only bylibqxcb.so, whichQT_QPA_PLATFORM=offscreennever loads.This was verified, not assumed: with the eight non-offscreen platform plugins physically deleted, the suite gave the same result (2 failed, 746 passed, 3 skipped). The reasoning is also recorded in a comment in the workflow step.
Follow-up work (outside this PR)
_DEJAVUis hardcoded toC:/Windows/Fonts/DejaVuSans.ttfintests/test_editor_text_fidelity.py:36, so 2 tests are skipped on Linux even though the font is available there.pymupdfhas no upper bound inrequirements.txt, which is how the 1.28.2 deprecation notice reached the--versiontest.Scope
One commit, three files:
.github/workflows/tests.yml(new),requirements-dev.txt,.gitignore. No application code, no tests, norequirements.txt, no version change. Reviewer and QA both approved.🤖 Generated with Claude Code