feat: support charmcraft.yaml format as meta for testing.Context by james-garner-canonical · Pull Request #2296 · canonical/operator

james-garner-canonical · 2026-01-30T04:06:01Z

This PR adds support for providing charmcraft.yaml formatted data to the meta argument of testing.Context.

It is an error to specify actions or config both as separate arguments and as keys in meta.

Resolves #1424

dimaqq

Looks good!

testing/tests/test_context.py

testing/src/scenario/context.py

Co-authored-by: Dima Tisnek <dimaqq@gmail.com>

This PR drops Scenario's build dependency on [setuptools_scm](https://pypi.org/project/setuptools-scm/), which we don't need, as it's for versioning a package based on git tags. This resolves the current build issue, example identified by David [here](https://github.com/canonical/operator/actions/runs/21809969678/job/62920050703#step:5:22). The build issue can be reproduced locally on `main` with `uv build --all`. I've verified that the build succeeds with `setuptools_scm` removed. The root cause of why builds started failing isn't clear though.

The Juju `credential-get` hook command is available for K8s models as of Juju 3.6.10, so adjust Scenario's consistency checker to match. Fixes canonical#2304

This is an automated PR to update the best practices documentation. Co-authored-by: David Wilding <david.wilding@canonical.com>

This PR prepares the release of version 3.5.2. --------- Co-authored-by: Tony Meyer <tony.meyer@gmail.com>

This PR updates the version files after the release.

Bumps `cryptography` from 46.0.2 to 46.0.5 to address [this security issue](https://github.com/canonical/operator/security/dependabot/23). The package is used by `pyjwt`, which is used by the release script, so we aren't really exposed by this issue, but it's simple to pull in the latest fixes and clear the security warning.

The 2.23 maintenance branch does not have the versions.md doc. Handle that when releasing.

This PR splits [How to run workloads with a Kubernetes charm](https://documentation.ubuntu.com/ops/latest/howto/run-workloads-with-a-charm-kubernetes/) into a subcategory of the how-to guides. I've redistributed all the content without making significant changes. We can do further improvements in follow-on PRs. Main parts to focus on reviewing: - The introductory descriptions on [Manage containers](https://canonical-ubuntu-documentation-library--2309.com.readthedocs.build/ops/2309/howto/manage-containers/) and the scope of each page in that subcategory. I moved "Manage Pebble metrics" into this subcategory because I feel it fits better here, rather than as a top-level how-to guide (this move isn't in the plan I prepared internally). - The same descriptions at [How-to guides > Managing containers](https://canonical-ubuntu-documentation-library--2309.com.readthedocs.build/ops/2309/howto/#managing-containers). I'm copying the approach of "Legacy guides" - listing each of the pages in the subcategory instead of only listing the subcategory itself. I'm open to changing the approach if we don't like the duplication. I also fixed `test_add_layer()` in [How-to guides > Manage the workload container > Write unit tests](https://canonical-ubuntu-documentation-library--2309.com.readthedocs.build/ops/2309/howto/manage-containers/manage-the-workload-container/#write-unit-tests). The test previously defined `container` but used `container_in`. I've switched the test to use `container_in` and `container_out` for clarity.

…nical#2315) Fixes canonical#2312 When `ops.hookcmds` has landed, we forgot to add the submodule to the setuptools config in pyproject.toml and now we get warnings. The PR addresses that.

As decided at the standup, bumping the default Juju version in Scenario Context to the version of the current Juju LTS, 3.6.14.

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

testing/src/scenario/context.py

dimaqq

Let's discuss 3x deepcopy.

testing/src/scenario/context.py

tonyandrewmeyer

I feel like it may be better to raise an error if there's an actions/config dict provided both in the meta argument and separately, rather than let one of them 'win'.

I wonder if we should add an example somewhere in the documentation (maybe in one of the library docs that isn't going to be moved to charmlibs?). In most cases, charms are ideally never using this and always auto-spec'ing. However, if they do use it, I would rather we encourage:

ctx = Context(MyCharm, charmcraft_dict)

rather than

ctx = Context(MyCharm, meta=charmcraft_dict)

And reserve the meta= keyword passing for when it's specifically the metadata dict.

I haven't tried running this since it's unclear what the exact implementation of the core piece will be, but I'm happy to approve trusting that it'll work at that point.

tonyandrewmeyer · 2026-02-18T05:41:52Z

testing/tests/test_context.py

+
+def test_init_with_bad_meta():
+    ctx = Context(MyCharm, meta={'a truth universally acknowledged': 'it'})
+    with pytest.raises((UncaughtCharmError, KeyError)):


With #2314 coming, it's probably easier to preemptively use that approach here.

TBH I think this is lower weight than adding monkeypatch and using monkeypatch.setenv.

testing/src/scenario/context.py

dimaqq · 2026-03-02T07:25:07Z

I feel like it may be better to raise an error if there's an actions/config dict provided both in the meta argument and separately, rather than let one of them 'win'.

Let's debate and make a decision on this.

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

james-garner-canonical · 2026-03-11T01:54:27Z

testing/tests/test_e2e/conftest.py

 @pytest.fixture
 def secrets_context(secrets_charm_meta: dict[str, Any]):
-    return Context(SecretsCharm, meta=secrets_charm_meta, actions=secrets_charm_meta['actions'])
+    return Context(SecretsCharm, meta=secrets_charm_meta)


Making it an error to pass actions both as a separate argument and as an entry in meta actually broke this fixture. It loads secrets_charm_meta from a charmcraft.yaml file, and passes ['actions'] explicitly (since it was written before this PR :)).

I think this is a sign this change might be too breaking.

Could we try it across our inventory of charms?

Good idea, I'm running it now, but I'm not confident in my ability to actually detect new failures from the output. Would you mind running it too?

I get the same number of successes with the branch, so it seems safe to me.

However, I'm open to making it a warning (maybe a deprecation warning?) instead if you think it's better to be safe.

Since this doesn't break any charm tests we can detect, let's keep it as an error.

james-garner-canonical · 2026-03-11T02:00:13Z

Requesting re-review as things have changed significantly:

deepcopy change landed separately, so this PR builds on that
We explicitly error if actions or config are passed in meta and separately at the same time -- but this actually broke some of our tests, so I wonder if this is too breaking a change.
- We could let the separate argument take precedence like it does in main, and perhaps warn in this case.
- Or try a gentler breaking change, like making it an error if they're passed separately and aren't identical.

tonyandrewmeyer

I'm fine with this, although I agree that there's some risk and I think we should investigate more what the impact might be. Perhaps we should only be warning when there's a collision.

tonyandrewmeyer · 2026-03-12T00:27:49Z

testing/tests/test_e2e/conftest.py

 @pytest.fixture
 def secrets_context(secrets_charm_meta: dict[str, Any]):
-    return Context(SecretsCharm, meta=secrets_charm_meta, actions=secrets_charm_meta['actions'])
+    return Context(SecretsCharm, meta=secrets_charm_meta)


Could we try it across our inventory of charms?

tonyandrewmeyer · 2026-03-12T01:13:14Z

testing/src/scenario/context.py

+            meta = copy.deepcopy(meta) if meta is not None else {'name': str(charm_type.__name__)}
+            if actions is not None and 'actions' in meta:
+                raise ValueError('Cannot specify actions in both charmcraft.yaml and separately')
+            actions = copy.deepcopy(actions) if actions is not None else meta.pop('actions', None)
+            if config is not None and 'config' in meta:
+                raise ValueError('Cannot specify config in both charmcraft.yaml and separately')
+            config = copy.deepcopy(config) if config is not None else meta.pop('config', None)
+            spec = _CharmSpec(charm_type=charm_type, meta=meta, actions=actions, config=config)


Freel free to push back on this; it's pretty subjective. I don't find this very readable.

Some of it maybe ruff pushes us into, like always using the ternary operator? If so then I would say that the consistency gained through tooling-enforced rules wins out.

Maybe a comment would help? Maybe if it was in a loop? (but it's only two items so ...)

I find my argument weakening by not being able to immediately see how I would write a nicer version of this.

Maybe comments to divide into logical sections and explain what's up? Maybe flipping the ternary to use is rather than is not?

# Metadata: may be in charmcraft.yaml format meta = {'name': str(charm_type.__name__)} if meta is None else copy.deepcopy(meta) # Actions: it is an error if actions is specified both separately and in charmcraft.yaml if actions is not None and 'actions' in meta: raise ValueError('Cannot specify actions in both charmcraft.yaml and separately') actions = meta.pop('actions', None) if actions is None else copy.deepcopy(actions) # Config: it is an error if config is specified both separately and in charmcraft.yaml if config is not None and 'config' in meta: raise ValueError('Cannot specify config in both charmcraft.yaml and separately') config = meta.pop('config', None) if config is None else copy.deepcopy(config) # Charm spec from user specified charm type and metadata spec = _CharmSpec(charm_type=charm_type, meta=meta, actions=actions, config=config)

Maybe:

meta = copy.deepcopy(meta) if meta else {'name': str(charm_type.__name__)} # actions if actions is None: actions = meta.pop('actions', None) elif 'actions' in meta: raise ValueError('Cannot specify actions in both charmcraft.yaml and separately') else: actions = copy.deepcopy(actions) # config if config is None: config = meta.pop('config', None) elif 'config' in meta: raise ValueError('Cannot specify config in both charmcraft.yaml and separately') else: config = copy.deepcopy(config) spec = _CharmSpec(charm_type=charm_type, meta=meta, actions=actions, config=config)

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

dimaqq · 2026-03-18T07:17:14Z

Sorry I lost track about what we decided for copying stuff exactly (same pr? separate? specific copies? whack-all deepcopy?).
Will put this on discussion tomorrow.

james-garner-canonical · 2026-03-18T07:30:39Z

Sorry I lost track about what we decided for copying stuff exactly (same pr? separate? specific copies? whack-all deepcopy?). Will put this on discussion tomorrow.

We decided on doing the deep copies in:

fix: hold only copies of user provided meta/config/actions in testing.Context #2349

This PR doesn't introduce any new copying, just pulls 'config' and 'actions' out of meta.

dimaqq · 2026-03-18T10:04:59Z

testing/src/scenario/context.py

-                actions=copy.deepcopy(actions),
-                config=copy.deepcopy(config),
-            )
+            meta = copy.deepcopy(meta) if meta is not None else {'name': str(charm_type.__name__)}


This is a subtle change for the case of explicit meta={} and supplying a non-empty actions or config.

I'm not sure why someone would do that, so I think it's OK.

I wonder if the code on line 691 should be changed to:

if not any(x is not None for x in (meta, actions, config)):

Though against, I've no clue why someone may override action or config with an empty dict (no actions or no config), and yet supply a not-None, yet-empty meta.

So maybe current code is fine?
Please double-check my logic.

You're right about the subtle behaviour change, and I'm not 100% sure why I made the decision to test meta for None instead of falsiness other than perhaps for symmetry with config and actions. I'll revert to the original behaviour for meta.

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

james-garner-canonical added 2 commits February 6, 2026 12:47

feat: have Haiku implement Dima's suggestion for charmcraft.yaml format

9919219

style: tox -e format

488c5fa

james-garner-canonical force-pushed the 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context branch from 6b900f1 to 488c5fa Compare February 5, 2026 23:47

james-garner-canonical added 6 commits February 6, 2026 13:40

test: rewrite tests manually

ffc7d07

test: add ctx.run to tests and add a test that fails there

6de08dc

test: use == instead of is for dict comparisons

d01128a

test: copy dicts and pop config and actions

39c475c

chore: fix linting

7bbf77c

docs: rewrite changes to docstring to be accurate

1915748

james-garner-canonical requested review from dimaqq and tonyandrewmeyer February 6, 2026 03:02

james-garner-canonical marked this pull request as ready for review February 6, 2026 03:02

dimaqq approved these changes Feb 6, 2026

View reviewed changes

testing/tests/test_context.py Show resolved Hide resolved

testing/tests/test_context.py Outdated Show resolved Hide resolved

dimaqq reviewed Feb 6, 2026

View reviewed changes

testing/src/scenario/context.py Show resolved Hide resolved

james-garner-canonical and others added 12 commits February 13, 2026 12:29

style: avoid unnecessary assignment

930cc69

Co-authored-by: Dima Tisnek <dimaqq@gmail.com>

fix: credential-get is available on k8s in newer Juju (canonical#2307)

e2049b8

The Juju `credential-get` hook command is available for K8s models as of Juju 3.6.10, so adjust Scenario's consistency checker to match. Fixes canonical#2304

chore: update best practices (canonical#2308)

910942b

This is an automated PR to update the best practices documentation. Co-authored-by: David Wilding <david.wilding@canonical.com>

chore: update changelog and versions for 3.5.2 release (canonical#2319)

534194e

This PR prepares the release of version 3.5.2. --------- Co-authored-by: Tony Meyer <tony.meyer@gmail.com>

chore: adjust versions after release (canonical#2320)

d19f2eb

This PR updates the version files after the release.

ci: fix releasing on branches with no versions.md doc (canonical#2323)

1f86266

The 2.23 maintenance branch does not have the versions.md doc. Handle that when releasing.

chore: include ops.hookcmds in Ops setuptools package discovery (cano…

e17e4bb

…nical#2315) Fixes canonical#2312 When `ops.hookcmds` has landed, we forgot to add the submodule to the setuptools config in pyproject.toml and now we get warnings. The PR addresses that.

feat: bump default Juju version in ops.testing.Context (canonical#2316)

53c7abe

As decided at the standup, bumping the default Juju version in Scenario Context to the version of the current Juju LTS, 3.6.14.

chore: merge main

2744b15

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

dimaqq reviewed Feb 17, 2026

View reviewed changes

testing/src/scenario/context.py Outdated Show resolved Hide resolved

dimaqq requested changes Feb 17, 2026

View reviewed changes

dimaqq reviewed Feb 18, 2026

View reviewed changes

testing/src/scenario/context.py Outdated Show resolved Hide resolved

tonyandrewmeyer approved these changes Feb 18, 2026

View reviewed changes

james-garner-canonical mentioned this pull request Feb 26, 2026

fix: hold only copies of user provided meta/config/actions in testing.Context #2349

Merged

james-garner-canonical added 6 commits March 11, 2026 09:52

chore: merge main

4cc5949

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

refactor: deepcopy where main does

77cc423

test: satisfy type checker

da9519b

feat: it is an error to pass actions/config in both meta and separately

d743470

test: set a good example by passing charmcraft yaml positionally

2d5ba58

chore: lint

f5cf785

james-garner-canonical commented Mar 11, 2026

View reviewed changes

james-garner-canonical requested review from dimaqq and tonyandrewmeyer March 11, 2026 01:57

tonyandrewmeyer approved these changes Mar 12, 2026

View reviewed changes

chore: merge main

c34b308

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

dimaqq approved these changes Mar 18, 2026

View reviewed changes

james-garner-canonical added 3 commits March 19, 2026 14:22

feat: test meta for truthiness rather than not None

c4ad4ec

chore: restructure checks for actions and config to improve readability

bfd36f5

chore: merge main

d444406

Merge branch 'main' into 26-01+feat+support-charmcraft-yaml-as-meta-for-testing-context

james-garner-canonical merged commit 64dde25 into canonical:main Mar 19, 2026
59 checks passed

Conversation

james-garner-canonical commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dimaqq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dimaqq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonyandrewmeyer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dimaqq commented Mar 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-garner-canonical commented Mar 11, 2026

Uh oh!

tonyandrewmeyer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaqq commented Mar 18, 2026

Uh oh!

james-garner-canonical commented Mar 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

james-garner-canonical commented Jan 30, 2026 •

edited

Loading