-
Notifications
You must be signed in to change notification settings - Fork 4
Add podman support #16
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
We also need to update the Github workflows to test the MacOS path, it needs a
runs-on: macos-latest in there, but the test of the steps should be identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so there's a few things to fix:
- The imports need formatting:
cmd [3] | ruff check --select I --show-fixes dbtesttools
dbtesttools/tests/test_pgfixture.py:1:1: I001 [*] Import block is un-sorted or un-formatted - The GH workflows are failing because Podman is not installed. This might help: https://github.com/marketplace/actions/install-podman
- We need to add the macos runner to the test matrix in GH workflows.
The good news is that the tests all pass for me locally \o/
|
I have no idea why your branch is failing the But there's a bunch of other error output from the tests, which are still passing, so something is not quite right. You also still need to make sure podman is installed and running. |
|
I suspect most of the errors are because podman is not up and running in the GH container that's running the tests. |
add podman support cleanup lint add socket verification add scenarios and prevent early termination errors add Podman related instructions ruff lint add macos target to github actions fixing typo add docker and podman runtimes to gh actions workflow switch to docker-ce on ubuntu split test on podam and docker into 2 separate jobs split podman and docker tests on macos Podman and docker can't coexist on macos. thus running it separately fixing path switch to colima to run docker cli handle podman startup only test podman on MacOs removing mess remove macos and separate podman and docker on ubuntu adjust path to tests bogus commit to rerun the workflow fix freaking twine error typo indentation indentation
| gnupg \ | ||
| lsb-release | ||
| sudo mkdir -p /etc/apt/keyrings | ||
| curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo gpg --dearmor -o /etc/apt/keyrings/docker.gpg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on curl here is very bad, it means the tests won't pass if that site is down or something similar is broken.
| # Override the scenarios to only run Podman | ||
| TestPostgresContainer.scenarios = [ | ||
| ("podman", {"podman": True}), | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you're doing this, and the previous change. The scenarios declaration will run both of these automatically. This change means that the last file to to get imported overrides the scenarios as well, so for example on my machine the only scenario that gets run now is podman.
When running hatch run ci locally it should do exactly the same tests as Workflows would run, which is why the initial workflow file was as simple as it was, with the addition of the Python test matrix (which should arguably be declared in the Hatch matrix, but that's on my list of things to fix later).
In other words, we should not depend on GH Actions to run all of our tests to completion.
Let's go back to basics with this change and simply run up a worker with podman installed. See here https://github.com/marketplace/actions/install-podman for one in the marketplace.
I don't think you need all that docker-ce stuff as well, was there a reason to do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah apparently that podman action is deprecated, crap. Back to manual installation.
|
Also it was not indentation that cause the RST rendering break, it was the lack of a blank line. Here's a patch for you: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made the changes I suggested and pushed a new branch up:
https://github.com/juledwar/db-testtools/pull/17/files
I also simplified the Actions config. You can copy the changes to your own branch or just merge mine.
|
Closing in favour of #17 |
This change is