Repository navigation
Run the test suite on Python 2.7 and 3.8 in GitHub Actions - #5
Conversation
The fork's only CI config was .travis.yml, which no longer runs, so pull requests reported no checks and security releases rested on local runs. Python 2.7 runs in the python:2.7 image because setup-python no longer provides it. tox.ini's addopts asks for pytest-cov, so the job clears it.
checkout@v4 targets Node.js 20, which GitHub now forces onto Node.js 24 with a deprecation warning. Python 3.8 is end of life, so a supported Python 3 joins the matrix beside it.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the added GitHub Actions test workflow.
No open issues 🎉
What changed in this PR
Adds GitHub Actions test coverage for the Python 2.7 maintenance branch, providing automated checks for future changes to the 4.5.post1 security release.
Changes:
- Runs tests on pull requests, pushes to
version-4.5.x, and manual dispatch. - Tests Python 2.7, 3.8, and 3.13 in official containers, with compatible dependency pins for Python 2.7.
- Overrides pytest’s default options to avoid requiring pytest-cov; leaves Travis configuration unchanged.
| File | Description |
|---|---|
.github/workflows/tests.yml |
Adds the container-based Python test matrix with read-only repository permissions. |
Review effort: Balanced
swagile1
left a comment
There was a problem hiding this comment.
Verdict: approve, no blockers. This does what PROD-2446 asks: a PR to version-4.5.x runs the suite on Python 2.7 and a supported Python 3 and reports a check.
Change map
Problem. The fork's only CI config is a dead .travis.yml, so the 4.5.post1 security release (#4) was verified only by local 2.7 runs. The PR body matches the ticket.
Approach. One file, .github/workflows/tests.yml: a two-entry matrix in python:2.7 (pytest 4.6.11, mock 3.0.5, pyasn1 0.4.8 pinned, since setup-python no longer ships 2.7) and python:3.8 (unpinned). Triggers on every PR, pushes to version-4.5.x, and workflow_dispatch; contents: read; -o addopts= drops tox.ini's coverage flags.
Evidence. Both jobs are green at 92 passed each, but the branch starts at 4834b90, before #4 merged, so those checks ran against the pre-fix code. I merged this PR onto the current version-4.5.x and ran the workflow's own commands in the same two images: 99 passed on 2.7, 99 passed on 3.8, matching the ticket's count. The pre-2.7.7 _compare_digest fallback is exercised directly by tests/test_compat.py:84-96, so the 2.7.18 image covers it.
Shape. No new code beyond the YAML, and no earlier GitHub Actions workflow in the fork to duplicate. Doctests stay out, as the ticket allows. .travis.yml and tox.ini are untouched.
What merging commits us to. Little; reverting is deleting one file. The 3.8 job can drift as dependencies drop 3.8, though pip's python_requires handling keeps choosing versions that support it.
Findings
🔴 Blockers: None.
⚪ Suggestions
- Rebase on
version-4.5.xand re-run[importance 1 / effort 1]. The checks and the body's "92 passed" predate #4. The merged tree passes 99/99 locally, but the PR's own record should show the security code under test. - Bump
actions/checkout@v4to@v5[importance 1 / effort 1]. Both jobs warn that Node.js 20 is deprecated and checkout@v4 is being forced onto Node 24. It already runs fine on Node 24 inside the buster-basedpython:2.7container, so v5 is safe. - No change needed here: the ticket's agent notes credit
setup.cfgwith the coverage flags, but they come fromtox.ini's[pytest] addopts, as the PR comment says.
Checked
- Security:
pull_request, notpull_request_target; read-only token; no third-party actions beyond checkout. - Failure modes: a pip install failure fails the job;
fail-fast: falsekeeps a 3.8 break from hiding the 2.7 result; thepython:2.7tag has been frozen since 2020. -e . --no-depsafter installing pyasn1 explicitly is consistent withsetup.py'sinstall_requires.
Not assessed
- Doctests (out of scope per the ticket).
- THR's review dispatcher was not run, since this PR is outside THR; the checklist was applied by hand on a scratch clone.
The fork's only CI config is
.travis.yml, which no longer runs, so pull requests show no checks. The 4.5.post1 security release (#4) was verified only by local Python 2.7 runs.This adds
.github/workflows/tests.yml. It runspytest testson every pull request and on pushes toversion-4.5.x:python:2.7container, with pytest 4.6.11, mock 3.0.5 and pyasn1 0.4.8.setup-pythonno longer provides 2.7.setup.pylists) and Python 3.13 (in support), in thepython:3.8andpython:3.13containers.tox.ini's[pytest] addoptsasks for pytest-cov, so the job passes-o addopts=..travis.ymlis left in place. Removing it is a separate change.Verification
version-4.5.xafter Release 4.5.post1: CVE-2020-25658 for the Python 2.7 line #4 (52e2215): 99 passed on 2.7, 3.8, 3.12 and 3.13.sep_idx_bad = sep_idx < 10changed to< 0inrsa/pkcs1.py, the 2.7 run failstests/test_pkcs1.py:243(DecryptionError not raised).b''.CI on this push (
dcbdfba): 99 passed on 2.7.18, 3.8.20 and 3.13.18, with no Node.js deprecation warning.Tracking: Linear PROD-2446.