freedomofpress / securedrop

GitHub repository for the SecureDrop whistleblower platform. Do not submit tips here!
https://securedrop.org/
Other
3.62k stars 688 forks source link

Integration testing for securedrop-admin #3341

Open redshiftzero opened 6 years ago

redshiftzero commented 6 years ago

Description

We accidentally introduced a regression in #3327 which was not caught by our automated tests (fortunately it was caught by manual QA). Also, it is difficult to fully test changes like those in #3340 without end-to-end integration tests.

For this reason, it's worth adding integration tests for securedrop-admin such that we can catch these kinds of bugs in the future.

User Stories

As a SecureDrop developer, I want integration testing of all user-facing applications such that I catch regressions before they go to production.

Child issues

redshiftzero commented 6 years ago

Hey @kushaldas - I believe you mentioned you were working on this - can you push up your WIP if so?

redshiftzero commented 6 years ago

As is evident with #3426, this is high priority. We should have a test git repo for the logic in ./securedrop-admin check_for_updates.

emkll commented 6 years ago

Perhaps a slightly simpler approach would be for a potential tester to fork the repo, and add a remote to the local repo, since the logic to fetch the tags will pull tags from all remotes. This will allow to push tags with higher version number (non rc) to test the check_for_updates logic.

eloquence commented 6 years ago

After the 0.9.0 release, a missing cwd parameter for a subprocess caused a security check to fail when securedrop-admin update is executed from the graphical updater, which acts largely as a user-friendly wrapper. (See https://github.com/freedomofpress/securedrop/pull/3796.)

As discussed briefly during today's extended standup, we should account for this in our automated testing approach to avoid a repeat episode of this exact problem (this does not absolve us from having to test the graphical updater, but would provide an additional layer of automated sanity checks).