charmed-kubernetes / pytest-operator

Apache License 2.0
6 stars 13 forks source link

Enable reading charm name from charmcraft.yaml #121

Closed weiiwang01 closed 5 months ago

weiiwang01 commented 6 months ago

Charmcraft has been updated to read metadata information directly from the charmcraft.yaml file without a separate metadata.yaml. Update to pytest-operator to read metadata information from charmcraft.yaml when required.

For further details, refer to the pull request https://github.com/canonical/charmcraft/pull/1126.

weiiwang01 commented 6 months ago

Thanks! A minor performance/organisation suggestion:

Since metadata.yaml is less likely to exist (where the tests run) in the future, I feel it should likely be the fallback, with craftcraft.yaml checked first. This is especially the case since we already have to check if charmcraft.yaml exists.

So roughly:

charmcraft_yaml_exists = charmcraft_path.exists()
if chamrcraft_yaml_exists:
    # get the name
else:
    metadata_parh = ...
    ...
if charmcraft_yaml_exists and layer_path.exists():
    ...

Gotcha, updated, thanks!

gregory-schiano commented 6 months ago

@tonyandrewmeyer can we get another round of review for that one please ? We've another PR requiring it

tonyandrewmeyer commented 6 months ago

@tonyandrewmeyer can we get another round of review for that one please ? We've another PR requiring it

Looks good to me, but I don't have write access here, so you'll need Adam or someone to review also.

weiiwang01 commented 5 months ago

@tonyandrewmeyer can we get another round of review for that one please ? We've another PR requiring it

Looks good to me, but I don't have write access here, so you'll need Adam or someone to review also.

@addyess could you help to review this pull request? Thanks!

addyess commented 5 months ago

Funny, i was going to come add this this week. Thanks for beating me to it.