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

Add smoketest (package installed, keyring/repo files in place) #9

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rocodes
Copy link
Contributor

@rocodes rocodes commented Jan 15, 2025

Closes #8

  • Add test that keyring/repo files are in place and rpmdb contains key
  • Add smoketest make target that supports containerized test
  • (WIP) add smoketest to CI
  • Update README to explain rpm key id and link to rpm release management docs

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small comments about the tests, but overall looks good!

# Basic acceptance testing for keyring package.
# Expects fedora environment.

RPM_QUERY_KEYRING_PACKAGE = ["rpm", "-q", "securedrop-workstation-keyring"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless these need to be reused, the tests would be easier to read if the commands were inlined where they get invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(reuse it a bit checking multiple different packages, but made it clearer what's happening at invocation)

tests/test_keyring.py Outdated Show resolved Hide resolved
@rocodes rocodes force-pushed the add-test-for-sanity branch 4 times, most recently from 01cf554 to fd42eb8 Compare January 16, 2025 22:41
@rocodes
Copy link
Contributor Author

rocodes commented Jan 16, 2025

Thank you for the feedback, if only I had pushed my extra changes bit min sooner! I've added support for running the tests in the container, addressed your comment re test failure, and (wip) adding github action support, although still figuring that part out.

I went back and forth about making the build-rpm and smoketest separate github actions, and now I'm inclined to make them separate, so that it's clear what part is failing. It's all kind of overkill for a repo that will hardly ever be updated. opinions welcome.

@rocodes
Copy link
Contributor Author

rocodes commented Jan 16, 2025

(The way I'm trying to shove systemd into the fedora container is unsurprisingly not ideal: System has not been booted with systemd as init system (PID 1). Can't operate. 🙃 I'll look into this tomorrow)

@rocodes rocodes force-pushed the add-test-for-sanity branch 2 times, most recently from bbd7f99 to 1d5f760 Compare January 17, 2025 15:24
@rocodes rocodes force-pushed the add-test-for-sanity branch 4 times, most recently from 4b00bdb to 2a7ecc8 Compare January 17, 2025 16:03
@rocodes rocodes force-pushed the add-test-for-sanity branch from 092eecb to a89bd5b Compare January 20, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add basic keyring sanity test (python)
2 participants