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

Migrate tests to GitHub Actions #496

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Migrate tests to GitHub Actions #496

merged 1 commit into from
Apr 23, 2024

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented Apr 12, 2024

Migrates make test and make reprotest tests to GitHub Actions and removes the CircleCI configuration.

Fixes #482.

Test plan

  • Visual review of configuration
  • Inspect test results to ensure same tests are running as previously

@eloquence eloquence force-pushed the migrate-lint-reprotest branch 16 times, most recently from 58aefa8 to 1eaa3ea Compare April 18, 2024 23:18
@eloquence eloquence changed the title [WIP] Migrate tests to GitHub Actions Migrate tests to GitHub Actions Apr 18, 2024
@eloquence eloquence marked this pull request as ready for review April 18, 2024 23:21
@eloquence
Copy link
Member Author

eloquence commented Apr 18, 2024

This is ready for review, but does not include make reprotest yet -- the test is runnable, but it failed on https://github.com/freedomofpress/securedrop-builder/actions/runs/8745779791/job/24001431722. I'd like to determine if that's an expected failure or if it's not running correctly for some reason. (And yeah, then we can add a nightly as well.)

On the other hand, the CircleCI config appears to be borked on this repo -- CircleCI is complaining about lack of SSH checkout key. I defer to the SD team if that's worth fixing before we've got the job switched over. If not, I can poke more at it Monday/Tuesday to finish things up.

@eloquence
Copy link
Member Author

Re: reprotest, I do see a lot of warning: Not a git repository. Use --no-index to compare two paths outside a working tree, so it does not look like an expected test failure but potentially a need to adapt the checkout logic in that test flow.

@eloquence eloquence force-pushed the migrate-lint-reprotest branch 4 times, most recently from 49e9a7e to 2558a16 Compare April 22, 2024 23:56
@eloquence
Copy link
Member Author

eloquence commented Apr 23, 2024

reprotest is passing now. 🎉 The issue was twofold:

  • We need to set git's safe.directory option for the git operations to succeed
  • As per CircleCI config, we need to avoid running tests as root due to tar reproducibility issues when run as root

This PR is also removing the CircleCI config, so CircleCI is complaining now, but if the rest of it looks good, we can remove the CircleCI config fully from this repo prior to merge.

name: install test requirements and run tests
command: |
adduser --system ci --ingroup root
sed -i -re "292s/^(\s+).*\$/\1return _.prepend_to_build_command_raw('')/" /usr/lib/python3/dist-packages/reprotest/build.py
Copy link
Member Author

Choose a reason for hiding this comment

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

This in-place patching goes back to 18770bd and, if it was still relevant at all, was not necessary for the wheel reproducibility checks, which do not use the reprotest package that is being patched here. The actual Debian reproducibility check has been migrated into securedrop-client and no longer uses reprotest.

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.

Awesome, LGTM! We will want to move these jobs to bookworm ASAP now (c.f. freedomofpress/securedrop-client#1958) but we can do that in a separate PR. Will poke infra for the CircleCI disconnect.

@eloquence eloquence merged commit 3ae85f5 into main Apr 23, 2024
4 checks passed
@eloquence eloquence deleted the migrate-lint-reprotest branch April 23, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Migrate this repository to GitHub Actions
2 participants