Re-enables paxtest in testinfra #5848
Conversation
|
In the spirit of "leave no trace," the script wrapping pytest ( |
|
Seemed to work as expected in #5609 (comment), agree that it'd be nice to automatically uninstall paxtest in the cleanup function after it's done. |
conorsch
force-pushed
the
1039-paxtest-checks-in-testinfra
branch
from
62a7fa7
to
b7e051c
Compare
|
Rebased on latest develop. I added a commit to clean up the package afterwards. Didn't use the bash trap, since that required futzing with Ansible outside of our wrappers within the Tails environment. Instead, I just used a try/finally block in the test itself. End result is the same: after test run, regardless of whether the paxtest failed or not, the package will be removed. Ready for re-review. |
conorsch
force-pushed
the
1039-paxtest-checks-in-testinfra
branch
from
b7e051c
to
3f0bd46
Compare
|
(Kicking CI) |
conorsch
force-pushed
the
1039-paxtest-checks-in-testinfra
branch
from
3f0bd46
to
6ddc535
Compare
|
Rebased on top of latest develop, to satisfy CI. |
|
Fails on a fresh install, as the apt cache has not been updated with |
| if not host.exists("/usr/bin/paxtest"): | ||
| warnings.warn("Installing paxtest to run kernel tests") | ||
| with host.sudo(): | ||
| host.run("apt-get install -y paxtest") |
There was a problem hiding this comment.
Assumes that paxtest is available via apt - which may not be the case until apt-get update has run at least once on the host. Running apt get update && apt-get install -y paxtest would work.
conorsch
force-pushed
the
1039-paxtest-checks-in-testinfra
branch
from
6ddc535
to
1630a05
Compare
| paxtest_results.split('\n')): | ||
| print(paxtest_diff) | ||
| assert paxtest_results == paxtest_expected | ||
| finally: |
There was a problem hiding this comment.
Paxtest isn't removed in this finally clause, it needs sudo.
There was a problem hiding this comment.
Good catch! Fixed.
These tests were skipped because we weren't installing paxtest by default. Let's just install it as part of the test run. Removes the skip-in-prod marker, so that these checks can be used as part of QA on hardware. Updates the logic to be distro-specific. Removes paxtest after test run We don't install it by default, and it's only useful in QA, so let's have the test-only dependency automatically cleaned up after install.
conorsch
force-pushed
the
1039-paxtest-checks-in-testinfra
branch
from
1630a05
to
14ea71e
Compare
Status
Ready for review
Description of Changes
Fixes #1039
These tests were skipped because we weren't installing paxtest by
default. Let's just install it as part of the test run. Removes
the skip-in-prod marker, so that these checks can be used as part of QA
on hardware. Updates the logic to be distro-specific.
Testing
CI should be passing on both platforms.
Try to run against hardware, or at least prodVMs, via the
./securedrop-admin verifyworkflow.Are we OK with installing
paxtestautomatically via tests? I'd think so, given that we do it manually as part of QA anyway.Deployment
Mostly QA, so developer-focused. Technically if an Admin runs
./securedrop-admin verify, the remote state will change becausepaxtestwill be installed now. That's acceptable, IMO, since otherwise the paxtest checks would have been skipped.Checklist