pgsql-io / multicorn2

http://multicorn2.org
Other
83 stars 17 forks source link

Run all test automation during PRs #46

Closed mfenniak closed 4 months ago

mfenniak commented 4 months ago

This PR adds automatic test execution to any future multicorn2 PRs via GitHub Actions. It also provides a new command-line interface to do the same for a local development environment. It runs tests in all combinations of Python 3.9 & 3.10, and PostgreSQL 12 & 13 -- eg. {py:3.9, pg:12}, {py:3.9, pg:13}, {py:3.10, pg:12}, {py:3.10, pg:13}

I've tested this build on a detached fork (https://github.com/mfenniak/multicorn-temp-githubactions-debug/actions). You can see an example of a failed test run, which I simulated to ensure that failures fail the build, here: https://github.com/mfenniak/multicorn-temp-githubactions-debug/actions/runs/9039353193/job/24842035056

The major factors to know about this change are...

Nix

The Nix package manager is an ideal tool for this kind of work because it allows testing across all these different environments and dependencies without having to "install" them into the system, and without using docker containers that prevent the use of debugging tools. Nix is available for any Linux distribution, as well as macOS, and Windows Subsystem for Linux, making it fairly accessible.

However, it is unfamiliar to many people. If no other developers are familiar with Nix, it becomes difficult to improve or maintain these changes. However, the existing capability to run these tests through make installcheck has been retained and unmodified and can still be used.

I mentioned all of this in #44 where I was inquiring in advance as to whether this patch would be a good idea... but I was too intrigued by the idea of building it to wait around for a response. 🤣

Disabled versions

Tests for Python 3.11+ and PostgreSQL 14+ are currently broken, so I've disabled them (see "testPythonVersions" and "testPostgresVersions" in flake.nix). I'd like to hack on fixing these, but I thought it would be best to keep functional or test changes separate from this continuous integration change.

Repo-level configuration / Build cache

As the existing tests use PL/Python, which is not enabled by default in Nix's version of PostgreSQL, I've had to build custom versions of PostgreSQL. This is easy to do in Nix, but, it is time-consuming for so many versions across so many Python dependencies. In order to mitigate this, I've added a binary cache to the GitHub Action -- an S3 bucket that is used to store the PostgreSQL versions that were previously built.

In order for that cache to function, three repository secrets need to be set; AWS_ACCESS_KEY_ID & AWS_SECRET_ACCESS_KEY to give the build access to the S3 bucket via a limited-permission user, and NIX_CACHE_SECRET_KEY in order to sign the artifacts to prevent tampering.

This S3 bucket should be very low cost to operate for this purpose, and so I'm happy to deploy it in my own AWS account and provide this access. However!, builds will not work until these secrets are configured on the GitHub repo.

The ideal way for me to configure these secrets would be if I could be added as a repo administrator, and then continue to hold that position as a collaborator in the future as I love this little project -- but I'd also be happy to share the secrets out-of-band and describe how to configure them, if that'd be your preference.

GitHub Actions

When new PRs are opened against this repo, it would be necessary to review and approve them to run the actions (https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks) in order to avoid any malicious code sneaking into the run_tests.yml file.

mfenniak commented 4 months ago

This PR is theoretically good-to-go -- the changes in main have been merged in, the GitHub Actions secrets have been configured in this repo, all tests pass when run locally, and a test run in my detached fork verified no problems (https://github.com/mfenniak/multicorn-temp-githubactions-debug/actions/runs/9044918373/job/24854099967).

I had previously deleted the docker-based infrastructure in this PR, but since @luss updated that for the version changes, I assume it's in-use so I reverted those changes.

It hasn't actually run in this repo -- I think that's because GitHub still thinks of it as an outside collaborator's PR that hasn't been approved to run -- but I assume it will run once merged and I'll quickly correct if any problems occur.

Positive 👍 on going ahead?

mfenniak commented 4 months ago

It's always revertible if someone wants to go down a different path... I've got a couple more hours this weekend to get the SQLAlchemy tests running but it's getting confusing without merging these PRs. :smiley:

luss commented 4 months ago

No chance we will reverse, great stuff!!

On Sun, May 12, 2024 at 8:26 PM Mathieu Fenniak @.***> wrote:

Merged #46 https://github.com/pgsql-io/multicorn2/pull/46 into main.

— Reply to this email directly, view it on GitHub https://github.com/pgsql-io/multicorn2/pull/46#event-12779817009, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMWOHTNBCKSU7GVMZ3EUI3ZCACDJAVCNFSM6AAAAABHRNMMWCVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSG43TSOBRG4YDAOI . You are receiving this because you were mentioned.Message ID: @.***>