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 regression workflow #131

Merged
merged 7 commits into from
Sep 12, 2024
Merged

add regression workflow #131

merged 7 commits into from
Sep 12, 2024

Conversation

antbern
Copy link
Contributor

@antbern antbern commented Sep 9, 2024

An attempt to create a CI workflow that can do some kind of regression test and compare the generate output map from the PR branch with the output of the master branch 🚀

Steps:

  1. Download a LAZ input file for use for the regressions. I personally uploaded a file to google drive and stored the download link in the REGRESSION_LAZ_FILE_URL secret. You will need to do something similar to make this work.
  2. Compile and run the program on the PR branch.
  3. Checkout the master branch.
  4. Compile and run the program on the master branch.
  5. Download, extract and run the binary from the latest release.
  6. Compare the generated output map files. I tried using imagemagick but it has some memory limits when running in the GitHub runners, so in the end I ended up with pngcomp which checks pixel-by-pixel and outputs some metrics.
  7. Upload the metrics and generated files as artifacts (in case one want to inspect the files)
  8. Check if some metric is below threshold for acceptance and fail the job if change is too big. Currently set to fail if there is not a 100% overlap between the two maps.

What do you think? 🤔


- name: Checkout master branch
run: |
git fetch origin master
Copy link
Owner

Choose a reason for hiding this comment

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

Could we use latest binary from release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should be doable yes. It depends on what should be tested: the latest release vs the current new features (PR branch), or the latest features (possibly unreleased) against the new features (PR branch) 🤔

Copy link
Owner

Choose a reason for hiding this comment

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

Let's test against latest release if possible, what is in master might not always be clean.
Also I am thinking it may help the test execution time if we do not have to compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 3badd60, see a successful workflow run here: https://github.com/antbern/rusty-pullauta/actions/runs/10808286676

I also added the output as a job summary which can also be seen in the link above 🚀

Copy link
Owner

Choose a reason for hiding this comment

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

Do you mind sharing the LAZ file you used in your test, I'll host it somewhere...

Copy link
Contributor Author

@antbern antbern Sep 11, 2024

Choose a reason for hiding this comment

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

Sure, here is a link. It is from Lantmäteriet and provided without license (CC0) so should be fine to host publicly 💯 It's about 180Mb and covers a 2,5km² area.

Perhaps there are better files with more varied terrain etc. Should probably also add a test that processes multiple files at some point...

@rphlo
Copy link
Owner

rphlo commented Sep 12, 2024

"""With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.""" Let's use a variable instead.
preference for name like KP_TEST_LAZ_URL

@rphlo rphlo merged commit 1537cda into rphlo:master Sep 12, 2024
4 of 5 checks passed
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.

2 participants