Skip to content

fix: add missing dependency (pylibjuju)#120

Open
tonyandrewmeyer wants to merge 1 commit intocanonical:mainfrom
tonyandrewmeyer:add-missing-juju-dep
Open

fix: add missing dependency (pylibjuju)#120
tonyandrewmeyer wants to merge 1 commit intocanonical:mainfrom
tonyandrewmeyer:add-missing-juju-dep

Conversation

@tonyandrewmeyer
Copy link

@tonyandrewmeyer tonyandrewmeyer commented Dec 10, 2023

The tests use importlib.metadata to get the installed version of pylibjuju (tests/conftest.py):

    if juju_version := os.environ.get("LIBJUJU_VERSION_SPECIFIER"):
        juju_version.replace("==", "")
        juju_version = juju_version[2:].split(".")[0]
    else:
        juju_version = version("juju")

However, pylibjuju is not installed as part of the requirements. This means that all tests fail with an error like:

ERROR tests/unit/test_data_interfaces.py::TestDatabaseProvides::test_database_requested_event - importlib.metadata.PackageNotFoundError: No package metadata was found for juju

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?

Copy link
Contributor

@shayancanonical shayancanonical left a 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!

Copy link
Contributor

@deusebio deusebio left a 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

@tonyandrewmeyer
Copy link
Author

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.

@deusebio
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants