close
Skip to content

feat: deprecate testing.Context.charm_spec#2219

Merged
james-garner-canonical merged 4 commits intocanonical:mainfrom
james-garner-canonical:25-12+feat+depreate-public-charm-spec-attribute
Dec 17, 2025
Merged

feat: deprecate testing.Context.charm_spec#2219
james-garner-canonical merged 4 commits intocanonical:mainfrom
james-garner-canonical:25-12+feat+depreate-public-charm-spec-attribute

Conversation

@james-garner-canonical
Copy link
Copy Markdown
Contributor

@james-garner-canonical james-garner-canonical commented Dec 5, 2025

This PR deprecates accessing testing.Context.charm_spec, because the _CharmSpec type is private, and was never intended to be exposed publicly.

I suggest we open PRs for the two known users of this attribute before the next release, or even before merging this PR, per @tonyandrewmeyer's research and suggested fixes:

  • grafana-agent-operator - this is using charm_spec to get the charm Class and adjust a private attribute on it. I would have thought the test could just import the charm class and do it directly - the charm_type isn't a copy, it's the same class object.
  • charmcraft (!) - I guess this is a legitimate use (so perhaps we shouldn't deprecate and maybe should document), pulling the container name from the metadata rather than hard-coding it in the test. The metadata isn't available elsewhere (you can get it from the model, but not until you've already called run). However, from_context() would do this for you in a nicer way, so maybe the right thing is to encourage that. (The test is using scenario as the namespace, so maybe it's quite old?).

Resolves #2131


Requesting review from @benhoyt, since his instinct was to avoid deprecation noise and publicly document this instead; and from @tonyandrewmeyer since he's the most familiar with this and what charms should be using instead.

@james-garner-canonical james-garner-canonical marked this pull request as ready for review December 5, 2025 03:35
@tonyandrewmeyer
Copy link
Copy Markdown
Collaborator

CharmSpec has:

  • the charm class - I feel like you should know what this is in your tests and just use it directly rather than via this object
  • the meta dict, which is roughly metadata.yaml
  • the actions dict, which is roughly actions.yaml
  • the config dict, which is roughly config.yml
  • an autoloaded Boolean - I feel this is definitely for internal use, and not something you'd use directly in tests.
  • a helper to get all the relations - it seems like no-one is using this, and it does seem like something that would make more sense exposed in a different way (as a general helper function or on the Context object) if people did want it

For the metadata, it seems like a meta property on Context that returned an ops.CharmMeta would be a cleaner way to provide this rather than the raw dictionaries. In my earlier (very quick) research it did seem like only charmcraft was actually using this. It does seem reasonable to want this information, but it also seems like in a lot of tests it'd be hard-coded and that isn't really an issue (and using from_context() is an alternative).

So I'm +0 on deprecating (and also opening a PR for grafana-agent-operator - maybe also charmcraft, although I think it's pinned to an old ops-scenario version, so there might be a need for a larger PR that modernises other things).

I'm interested in @PietroPasotti's take here too. Essentially: do you think we should document Context.charm_spec or deprecate it?

Copy link
Copy Markdown
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.

Assuming that we do deprecate, the code change looks good to me.

I think we should add a test that validates that there's a warning (pytest.warns) if you access .charm_spec. We probably don't need other tests, assuming that none of them are using .charm_spec directly (if so, we should pytest.warns there to silence the warning, but also see if that's actually a use-case).

Comment thread testing/src/scenario/context.py Outdated
@james-garner-canonical
Copy link
Copy Markdown
Contributor Author

james-garner-canonical commented Dec 14, 2025

I've opened PRs to migrate the two users off Context.charm_spec.

I'll update this PR to use @warnings.deprecated and to mention alternatives in the docstring before requesting further review.

Merge branch 'main' into 25-12+feat+depreate-public-charm-spec-attribute
Comment thread testing/tests/test_context.py
Comment thread testing/pyproject.toml
dependencies = [
"ops==3.5.0.dev0",
"PyYAML>=6.0.1",
"typing_extensions>=4.9.0", # for `deprecated` decorator, replace with stdlib `warnings.deprecated` when we're Python 3.13+
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warnings.deprecated is Python 3.13+. Luckily, typing_extensions has backported it, and type checkers respect the backport. This introduces a testing-only runtime dependency on typing_extensions (currently it's type checking only in ops), which I think is OK.

@james-garner-canonical
Copy link
Copy Markdown
Contributor Author

Turns out it needs to be typing_extensions.deprecated instead since warnings.deprecated is Python 3.13+. I've added a test to assert the warning is raised at runtime. The docstring and warning show up nicely in the IDE -- in VS Code, ctx.charm_spec is highlighted as an error, and the hover contents show:

Screenshot from 2025-12-15 10-22-02

Copy link
Copy Markdown
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.

Nice, thanks!

Comment thread testing/src/scenario/context.py Outdated
@james-garner-canonical james-garner-canonical merged commit a407ea4 into canonical:main Dec 17, 2025
55 of 56 checks passed
james-garner-canonical added a commit that referenced this pull request Dec 18, 2025
This PR fixes a linting error in the test newly added in #2219, which
is needed due to the linting updates in #2228.
lengau pushed a commit to canonical/charmcraft that referenced this pull request Dec 19, 2025
…xt.charm_spec (#2527)

The `ops.testing` framework currently exposes the private `_CharmSpec`
class via the `Context.charm_spec` attribute
(canonical/operator#2131). Charm Tech will
deprecate this attribute soon
(canonical/operator#2219). Charmcraft currently
uses `charm_spec` in the `init-extensions` tests, so this PR
preemptively modernises the test to use `State.from_context`, avoiding
the need to access `Context.charm_spec`.

The `init-extensions` tests run only in the Ubuntu 22.04 spread tests.
While that job is currently failing, the failing tests do not appear to
be the `init-extensions` tests:

```
2025-12-12 04:44:41 Successful tasks: 50
error: unsuccessful run
2025-12-12 04:44:41 Aborted tasks: 0
2025-12-12 04:44:41 Failed tasks: 3
    - google:ubuntu-22.04-64:tests/spread/smoketests/different-dir
    - google:ubuntu-22.04-64:tests/spread/smoketests/multi-base:basic
    - google:ubuntu-22.04-64:tests/spread/smoketests/multi-base:one_platform
2025-12-12 04:44:41 Failed task restore: 2
    - google:ubuntu-22.04-64:tests/spread/smoketests/multi-base:basic
    - google:ubuntu-22.04-64:tests/spread/smoketests/multi-base:one_platform
```

<details>
<summary>AFAICT the init-extensions tests pass, here's some relevant
output from <a
href=https://github.com/canonical/charmcraft/actions/runs/20154524656/job/57855410223?pr=2527>this
run</a>.</summary>

```
2025-12-12 03:40:14 Restoring google:ubuntu-22.04-64:tests/spread/commands/init-extensions:springboot (dec120311-758761)...
2025-12-12 03:40:28 Preparing google:ubuntu-22.04-64:tests/spread/smoketests/reactive:stable (dec120311-758761)...
2025-12-12 03:40:29 Executing google:ubuntu-22.04-64:tests/spread/smoketests/reactive:stable (dec120311-758761) (7/53)...
2025-12-12 03:40:34 Restoring google:ubuntu-22.04-64:tests/spread/commands/init-extensions:flask (dec120311-758749)...
2025-12-12 03:40:50 Preparing google:ubuntu-22.04-64:tests/spread/smoketests/reactive:two (dec120311-758749)...
2025-12-12 03:40:51 Executing google:ubuntu-22.04-64:tests/spread/smoketests/reactive:two (dec120311-758749) (8/53)...
2025-12-12 03:41:14 Restoring google:ubuntu-22.04-64:tests/spread/commands/init-extensions:expressjs (dec120311-758638)...
2025-12-12 03:41:14 Restoring google:ubuntu-22.04-64:tests/spread/commands/init-extensions:fastapi (dec120311-758711)...
2025-12-12 03:41:30 Preparing google:ubuntu-22.04-64:tests/spread/dependencies/ (dec120311-758711)...
2025-12-12 03:41:30 Preparing google:ubuntu-22.04-64:tests/spread/dependencies/ (dec120311-758638)...
2025-12-12 03:41:34 Restoring google:ubuntu-22.04-64:tests/spread/commands/init-extensions:go (dec120311-758738)...
2025-12-12 03:41:52 Preparing google:ubuntu-22.04-64:tests/spread/smoketests/multi-base:all (dec120311-758738)...
2025-12-12 03:41:55 Executing google:ubuntu-22.04-64:tests/spread/smoketests/multi-base:all (dec120311-758738) (9/53)...
2025-12-12 03:42:40 Restoring google:ubuntu-22.04-64:tests/spread/commands/init-extensions:django (dec120311-758757)...
```
</details>

<!-- Describe your changes -->
---

- [x] I've followed the [contribution
guidelines](https://github.com/canonical/charmcraft/blob/main/CONTRIBUTING.md).
- [x] I've signed the [CLA](http://www.ubuntu.com/legal/contributors/).
- [x] I've successfully run `make lint && make test`.
- [x] ~I've added or updated any relevant documentation.~ (N/A)
- [x] ~I've updated the relevant release notes.~ (N/A)
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.

Deprecate or document testing.Context.charm_spec

4 participants