-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix: add missing dependency (pylibjuju) #120
base: main
Are you sure you want to change the base?
fix: add missing dependency (pylibjuju) #120
Conversation
23fc6fb
to
f5a9071
Compare
…clash with the integration test stuff.
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.
TIL: tox env var substitutions. LGTM!
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.
Looks good! I would just update the reference to the juju to something more recent than 2.9.x
It's the same as you have for your integration tests (I assumed you'd want it to match), but happy to change it to whatever you'd prefer. |
We tests a number of versions, among which 2.9.42.4 which is the oldest. I would just keep on testing on CI on that one, but I would move the default reference to latest, e.g. 3.5.0.0. Otherwise the reference would already seem out of date. I don't think that should break anything, transitive dependencies included in tox.ini are not resolved in charmlib anyway... :D |
The tests use importlib.metadata to get the installed version of pylibjuju (tests/conftest.py):
However, pylibjuju is not installed as part of the requirements. This means that all tests fail with an error like:
This is fixed by installing pylibjuju for the unit environment as well as integration. Alternatively, the
version("juju")
could be wrapped in a try/except, and you could fall back to a hard-coded version?