SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
721 stars 168 forks source link

Run the tests automatically via Github Actions Workflow #469

Closed dbast closed 1 year ago

dbast commented 1 year ago

This adds a GitHub Action workflow to run the automated tests via pytest. GitHub Action workflows are freely available for OpenSource Projects and can be specified via yaml files in the repo.

The PR does the most simple thing by setting up Python, installing the dependencies and running the tests bare metal on the ubuntu-latest x86_64 runners, which makes it pretty fast.

Automatic test runs could make PR reviews easier to some extend :)

Here a link to the actions run in the fork of the repo: https://github.com/dbast/seedsigner/actions/runs/6063280014/job/16450424545

TODO: This requires somebody with permissions approving the Action run for this PR (due to new contribution) or maybe go to the Actions tab / settings of the repo and enable/allow actions.

jdlcdl commented 1 year ago

I really like!

Some thoughts to consider:

None of my comments above are to be interpreted as a NACK. In the spirit of "FOSS is a slow, sometimes dreadfully slow, march towards better." I fully support this as "better" and future pr's could continue that march.

ACK!!!!

dbast commented 1 year ago

@jdlcdl Added coverage reporting + more pytest options (like -vv, durations of the slowest 5 tests). The coverage is printed to the test log output, but also a html report is generated and uploaded.

You can see a full run here https://github.com/dbast/seedsigner/actions/runs/6065958918 ... click on the job to see the log or on the ci-artifacts to download the html report.

I am also happy to add running ./tests/screenshot_generator/generator.py and also storing that as downloadable artifact. Tried it and it fails not finding seedsigner-icons.ttf. Guess I have to do a more extensive font setup to get the generator.py files run correctly. Any hints?

jdlcdl commented 1 year ago

Tried it and it fails not finding seedsigner-icons.ttf. Guess I have to do a more extensive font setup to get the generator.py files run correctly. Any hints?

When setting up an RPi for development work, I usually follow the "Manual Installation". You'll notice much of this is geared particularly for raspberry-pi, but since the last release 0.6.0, much has been separated so that a non-RPi development computer can run the unit tests and the screenshot generator. It's possible that documentation is not complete and/or screenshot_generator imports are not sufficient, since this capability on non-RPi is recent. Perhaps follow that same document while ignoring RPi steps. I see that you're installing both requirements.txt files and not the requirements-raspi.txt, also seedsigner itself into the environment; this is good. The sudo apt imports are fewer than I expected, and I don't understand the open-sans fonts install. All fonts necessary for seedsigner should be in ./src/seedsigner/resources/fonts/ in the repo... and the two which I'm familiar with are .otf files.

dbast commented 1 year ago

@jdlcdl Figured it out:

So this PR now:

  1. runs the tests (red / green test status for every new PR after this is merged)
  2. print the coverage to the pytest log and also produces a html coverage report
  3. generates the screenshots
  4. uploads html coverage report + screenshots

A full run with the downloadable ci-artifacts can be currently found in my fork, see https://github.com/dbast/seedsigner/actions/runs/6070632443 ... yay, all the screenshots are there :)

kdmukai commented 1 year ago

@dbast wow, that's amazing progress! Very cool.

Now I'm getting greedy:

fyi: After v0.7.0 is released, @newtonick and I have a bit of downtime planned (~2-3wks for me) to catch up on other things, provided there are no urgent bugs to attend to.

But this PR is definitely an important addition and hopefully we can give it more attention when we're back.

dbast commented 1 year ago

@kdmukai While there is no direct possibility to make the artifacts browsable, there are other things that could help:

Would do this in a separate PR as the goal of this PR is to get the tests runnings in a fully automated way with as little as possible changes to the existing code base.

jdlcdl commented 1 year ago

I believe that there are already reasonable diffs of my cloned seedsigner_screenshots repo, as long as I generate them , commit and push them alongside code changes. Images are so pixel-for-pixel correct that the differences are reserved to where an intended code change actually altered a screen, exactly what we want, with an exception for some screens where the screenshot generator is randomizing inputs ie: seed backup words. In a perfect run, all pngs get recreated with a fresh timestamp, but only ones that actually changed are seen as 'modified' by git.

newtonick commented 1 year ago

ACK