Skip to content
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

chore: rename charms.yaml file to interface.yaml #18

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

IronCore864
Copy link
Collaborator

@IronCore864 IronCore864 commented Jun 20, 2024

In "overhaul to better support charmhub's interface pages", all charms.yaml are renamed to interface.yaml. Update pytest-interface-tester to load interface.yaml instead of charms.yaml.

See more discussion here.

Manual tests are done by installing this package from local path:

$ python3 run_matrix.py --include saml
Failed to load module test_requirer: No module named 'test_requirer'
INFO:root:Running tests for interface: saml
INFO:root:Running tests for version: v0
INFO:root:Running tests for role: provider
INFO:root:Running 1 saml interface tests on: ['saml-integrator']...
INFO:root:Running tests for saml
INFO:root:Running tests for charm: saml-integrator
INFO:root:Preparing testing environment for: saml-integrator
INFO:root:Cloning: saml-integrator from (https://github.com/canonical/saml-integrator-operator@main)
INFO:root:Preparing venv for /tmp/charm-relation-interfaces-tests/saml-integrator
INFO:root:Installing dependencies in venv for /tmp/charm-relation-interfaces-tests/saml-integrator
INFO:root:Installed lxml==4.9.4
ops==2.14.0
pydantic==1.10.16
signxml==3.2.2

INFO:root:Generating test file for saml at /tmp/charm-relation-interfaces-tests/saml-integrator/tests/interface
INFO:root:Running tests for /tmp/charm-relation-interfaces-tests/saml-integrator
You seem to be using pydantic v1. Please upgrade to v2, as compatibility may be dropped in a future version of pytest-interface-tester.
=================================================================== test session starts ====================================================================
platform darwin -- Python 3.11.8, pytest-8.2.2, pluggy-1.5.0
rootdir: /tmp/charm-relation-interfaces-tests/saml-integrator
configfile: pyproject.toml
plugins: interface-tester-2.0.1
collected 1 item

../../../../tmp/charm-relation-interfaces-tests/saml-integrator/tests/interface/interface_test_saml.py .                                             [100%]

==================================================================== 1 passed in 1.64s =====================================================================
INFO:root:Result: PASSED
INFO:root:Running tests for role: requirer
INFO:root:No tests specified for saml/requirer; skipping...
+++ Results +++
{
  "saml": {
    "v0": {
      "provider": {
        "saml-integrator": true
      },
      "requirer": {}
    }
  }
}

Note: We probably need to release a new version for this module but I'm not sure about the version: It's a breaking change, so by SEMVER it shouldn't be v2.0.2, although it seems "right" since it's a minor change... I left the version string in pyproject.toml unchanged, still 2.0.1 now.

Copy link
Collaborator

@tonyandrewmeyer tonyandrewmeyer 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 to me, although I'm not super familiar with this.

In regards to the version question, if we're not going to support both names, then a major bump seems the correct thing to do. It allows pinning to work as expected.

Note that you do need to have the version change in some way, because there's a workflow that does a release on merge to main.

interface_tester/collector.py Outdated Show resolved Hide resolved
interface_tester/collector.py Show resolved Hide resolved
Copy link
Collaborator

@PietroPasotti PietroPasotti 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 to me.

As for the version bump, I think we're the only ones depending on this so probably we could get away with a minor bump without breaking anyone's code, but still, my personal philosophy is: versions are free. So let's give it a major bump for the sake of hygiene :)

@IronCore864
Copy link
Collaborator Author

Thanks all for the review, I have fixed the naming and bumped the version to 3.0.0 in pyproject.toml.

@IronCore864
Copy link
Collaborator Author

A gentle reminder: Could you please merge it? @PietroPasotti

I don't have permission for this repo. Thanks!

@PietroPasotti PietroPasotti merged commit e2322ec into canonical:main Jul 30, 2024
2 checks passed
@PietroPasotti
Copy link
Collaborator

Sorry! was on holiday some time :)

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.

4 participants