test: Migrate integration tests from pytest-operator to jubilant#201
test: Migrate integration tests from pytest-operator to jubilant#201tonyandrewmeyer wants to merge 11 commits into
Conversation
swetha1654
left a comment
There was a problem hiding this comment.
LGTM, its perfect thank you for your contribution!
Replace pytest-operator/python-libjuju with jubilant and pytest-jubilant for integration tests. All async patterns converted to synchronous. Generated with GitHub Copilot (Claude Sonnet 4.6).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use @pytest.mark.usefixtures instead of a function parameter since traefik_app is only needed for its side effect (deploying traefik). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Juju treats bare filenames like "hockeypuck-k8s_amd64.charm" as ambiguous (could be charmhub name). Using resolve() gives an absolute path that Juju recognizes as a local file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8a783f6 to
4d9ac3f
Compare
|
I've rebased this and signed the commits, and also handled the changes that pytest-operator 2.0 brought since I opened this. I think the failures are some sort of transient issue unrelated here. I can't re-run to be sure. Please let me know if there's anything further needed from me here -- otherwise I'll leave this with you :). |
| "--hockeypuck-image", action="store", help="Hockeypuck app image to be deployed" | ||
| ) | ||
| parser.addoption("--charm-file", action="store", help="Charm file to be deployed") | ||
| # Compat shim: operator-workflows passes --keep-models, which was renamed |
There was a problem hiding this comment.
As noted here, this is a bit of a hack to work around the change from --keep-models to --no-juju-teardown. It really needs a change upstream (which is probably a breaking change, so major release for the action?), which seems out of scope here. The hack should work in the meantime.
cc @james-garner-canonical in case you have any better suggestion for this.
| """Return the current testing juju model.""" | ||
| assert ops_test.model | ||
| return ops_test.model | ||
| def pack(root: Path | str = "./", platform: str | None = None) -> Path: |
There was a problem hiding this comment.
FYI pack() was removed from pytest-jubilant. Our suggestion is that packing is done outside of the tests (this will work better when you're using charmcraft test, and also for other reasons). However, that seemed like too large a change for me to propose here, so I've just added a local pack() instead. I do recommend that you consider packing before the tests, though (see the release notes for pytest-jubilant 2.0 for more details, or reach out in Charm Development on Matrix).
Applicable spec: N/A
Overview
This PR migrates the integration tests from pytest-operator and python-libjuju to Jubilant and pytest-jubilant.
Note that this was originally written by AI as part of exploratory work by Charm Tech to see how AI (particularly copilot) can help get charms migrated to Jubilant. This PR is an example of that work (a post will appear on Discourse very soon referencing this PR and will more details). However, I have also reviewed it and made tweaks, so am putting myself as the human in the loop, so to speak.
However, what is needed is review from someone who is familiar with the charm and its tests, which I am not.
I'm opening this against the upstream repo as the CI can't run in my fork, since it needs hosted runners. This will allow verifying that the tests do pass.
Rationale
Using Jubilant allows for simpler and more modern and maintainable tests. Most importantly, it allows running the integration tests against both Juju 3 and Juju 4.
Juju Events Changes
N/A
Module Changes
Only test code is changed.
Library Changes
N/A
Checklist
The documentation for charmhub is updatedurgent,trivial,complex)I don't think I can label a PR as an external contributor. Let me know if this sort of change should have a changelog entry and I can add one.