Skip to content

Commit

Permalink
feat!: unify run() and run_action() and simplify context managers (#162)
Browse files Browse the repository at this point in the history
The `run_action()` method (both standalone and in the context manager)
are removed. This means that all events, including action events, are
emitted with the same `run()` call, and both return the output state.

To get access to the results of actions, a new `action_output` attribute
is added to the `Context`. This is a simplified representation of the
Juju `operation` object (and the `task` objects in them), which are
displayed with `juju run`, but also available via `juju show-operation`.
The class has the same name as the Harness `ActionOutput` and will be
consolidated into a single class when Scenario is added to ops.

For example:

```python
# Scenario 6
out = ctx.run_action(Action("backup", params={"output": "data.tar.gz"}), State())
assert out.results == {"size": 1234}
assert out.logs = [..., ...]
assert out.state...

# Scenario 7
state = ctx.run(ctx.on.action("backup", params={"output": "data.tar.gz"}), State())
assert ctx.action_output.results == {"size": 1234}
assert ctx.action_output.logs = [..., ...]
assert state...
```

When the charm calls `event.fail()`, this raises an exception, in the
same way that Harness does. For example:

```python
# Scenario 6
out = ctx.run_action("bad-action", State())
assert out.failure == "couldn't do the thing"

# Scenario 7
with pytest.raises(ActionFailed) as exc_info:
    ctx.run(ctx.on.action("bad-action"), State())
assert exc_info.value.message == "couldn't do the thing"
```

The advantage of this is that tests that the action is successful do not
need to have `assert ctx.action_output.status != "failed"`, which is
easy to miss.

In addition, the `Context.manager` and `Context.action_manager` methods
are replaced by the ability to use the `Context` object itself as a
context manager.

For example:

```python
ctx = Context(MyCharm)
with ctx(ctx.on.start(), State()) as event:
    event.charm.prepare()
    state = event.run()
    assert event.charm...
```

The same code is also used (with `ctx.on.action()`) for action events.

Advantages:
* Slightly shorter code (no ".manager" or ".action_manager")
* Avoids naming complications with "manager", "action_manager" and the
various alternatives proposed in #115.

The `.output` property of the context manager is also removed. The
context manager will still handle running the event if it's not done
explicitly, but if that's the case then the output is not available. We
want to encourage people to explicitly run the event, not rely on the
automated behaviour - although I think it does make sense that it does
run, rather than raise or end in a weird situation where the event never
ran.

This replaces #115 and #118, being a combination of ideas/discussion
from both, plus incorporating the unification of run/run_action
discussed here, and the "action fail raises" discussed elsewhere.

Also, as drive-by changes while names are being made consistent:
* `_Port` becomes `Port`
* `_RelationBase` becomes (again) `RelationBase`

---------

Co-authored-by: PietroPasotti <[email protected]>
  • Loading branch information
tonyandrewmeyer and PietroPasotti authored Aug 8, 2024
1 parent ff0e4e8 commit 2894855
Show file tree
Hide file tree
Showing 23 changed files with 355 additions and 377 deletions.
56 changes: 34 additions & 22 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ If you want to, you can override any of these relation or extra-binding associat

```python
state = scenario.State(networks={
scenario.Network("foo", [BindAddress([Address('192.0.2.1')])])
scenario.Network("foo", [scenario.BindAddress([scenario.Address('192.0.2.1')])])
})
```

Expand Down Expand Up @@ -726,8 +726,8 @@ storage = scenario.Storage("foo")
# Setup storage with some content:
(storage.get_filesystem(ctx) / "myfile.txt").write_text("helloworld")

with ctx.manager(ctx.on.update_status(), scenario.State(storages={storage})) as mgr:
foo = mgr.charm.model.storages["foo"][0]
with ctx(ctx.on.update_status(), scenario.State(storages={storage})) as manager:
foo = manager.charm.model.storages["foo"][0]
loc = foo.location
path = loc / "myfile.txt"
assert path.exists()
Expand Down Expand Up @@ -924,9 +924,9 @@ import pathlib

ctx = scenario.Context(MyCharm, meta={'name': 'juliette', "resources": {"foo": {"type": "oci-image"}}})
resource = scenario.Resource(name='foo', path='/path/to/resource.tar')
with ctx.manager(ctx.on.start(), scenario.State(resources={resource})) as mgr:
with ctx(ctx.on.start(), scenario.State(resources={resource})) as manager:
# If the charm, at runtime, were to call self.model.resources.fetch("foo"), it would get '/path/to/resource.tar' back.
path = mgr.charm.model.resources.fetch('foo')
path = manager.charm.model.resources.fetch('foo')
assert path == pathlib.Path('/path/to/resource.tar')
```

Expand Down Expand Up @@ -988,7 +988,6 @@ class MyVMCharm(ops.CharmBase):
An action is a special sort of event, even though `ops` handles them almost identically.
In most cases, you'll want to inspect the 'results' of an action, or whether it has failed or
logged something while executing. Many actions don't have a direct effect on the output state.
For this reason, the output state is less prominent in the return type of `Context.run_action`.

How to test actions with scenario:

Expand All @@ -1000,18 +999,32 @@ def test_backup_action():

# If you didn't declare do_backup in the charm's metadata,
# the `ConsistencyChecker` will slap you on the wrist and refuse to proceed.
out: scenario.ActionOutput = ctx.run_action(ctx.on.action("do_backup"), scenario.State())
state = ctx.run(ctx.on.action("do_backup"), scenario.State())

# You can assert action results, logs, failure using the ActionOutput interface:
assert out.logs == ['baz', 'qux']

if out.success:
# If the action did not fail, we can read the results:
assert out.results == {'foo': 'bar'}
# You can assert on action results and logs using the context:
assert ctx.action_logs == ['baz', 'qux']
assert ctx.action_results == {'foo': 'bar'}
```

## Failing Actions

If the charm code calls `event.fail()` to indicate that the action has failed,
an `ActionFailed` exception will be raised. This avoids having to include
success checks in every test where the action is successful.

```python
def test_backup_action_failed():
ctx = scenario.Context(MyCharm)

with pytest.raises(ActionFailed) as exc_info:
ctx.run(ctx.on.action("do_backup"), scenario.State())
assert exc_info.value.message == "sorry, couldn't do the backup"
# The state is also available if that's required:
assert exc_info.value.state.get_container(...)

else:
# If the action fails, we can read a failure message:
assert out.failure == 'boo-hoo'
# You can still assert action results and logs that occured as well as the failure:
assert ctx.action_logs == ['baz', 'qux']
assert ctx.action_results == {'foo': 'bar'}
```

## Parametrized Actions
Expand All @@ -1024,7 +1037,7 @@ def test_backup_action():

# If the parameters (or their type) don't match what is declared in the metadata,
# the `ConsistencyChecker` will slap you on the other wrist.
out: scenario.ActionOutput = ctx.run_action(
state = ctx.run(
ctx.on.action("do_backup", params={'a': 'b'}),
scenario.State()
)
Expand Down Expand Up @@ -1130,7 +1143,7 @@ Scenario is a black-box, state-transition testing framework. It makes it trivial
B, but not to assert that, in the context of this charm execution, with this state, a certain charm-internal method was called and returned a
given piece of data, or would return this and that _if_ it had been called.

Scenario offers a cheekily-named context manager for this use case specifically:
The Scenario `Context` object can be used as a context manager for this use case specifically:

```python notest
from charms.bar.lib_name.v1.charm_lib import CharmLib
Expand All @@ -1152,8 +1165,7 @@ class MyCharm(ops.CharmBase):

def test_live_charm_introspection(mycharm):
ctx = scenario.Context(mycharm, meta=mycharm.META)
# If you want to do this with actions, you can use `Context.action_manager` instead.
with ctx.manager("start", scenario.State()) as manager:
with ctx(ctx.on.start(), scenario.State()) as manager:
# This is your charm instance, after ops has set it up:
charm: MyCharm = manager.charm

Expand All @@ -1174,8 +1186,8 @@ def test_live_charm_introspection(mycharm):
assert state_out.unit_status == ...
```

Note that you can't call `manager.run()` multiple times: the manager is a context that ensures that `ops.main` 'pauses' right
before emitting the event to hand you some introspection hooks, but for the rest this is a regular scenario test: you
Note that you can't call `manager.run()` multiple times: the object is a context that ensures that `ops.main` 'pauses' right
before emitting the event to hand you some introspection hooks, but for the rest this is a regular Scenario test: you
can't emit multiple events in a single charm execution.

# The virtual charm root
Expand Down
1 change: 1 addition & 0 deletions docs/custom_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,5 +311,6 @@ def _compute_navigation_tree(context):
('py:class', '_Event'),
('py:class', 'scenario.state._DCBase'),
('py:class', 'scenario.state._EntityStatus'),
('py:class', 'scenario.state._Event'),
('py:class', 'scenario.state._max_posargs.<locals>._MaxPositionalArgs'),
]
1 change: 0 additions & 1 deletion docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ scenario.Context

.. automodule:: scenario.context


scenario.consistency_checker
============================

Expand Down
8 changes: 6 additions & 2 deletions scenario/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/env python3
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.
from scenario.context import ActionOutput, Context
from scenario.context import Context, Manager
from scenario.state import (
ActionFailed,
ActiveStatus,
Address,
BindAddress,
Expand All @@ -21,6 +22,7 @@
Network,
Notice,
PeerRelation,
Port,
Relation,
Resource,
Secret,
Expand All @@ -37,7 +39,7 @@
)

__all__ = [
"ActionOutput",
"ActionFailed",
"CheckInfo",
"CloudCredential",
"CloudSpec",
Expand All @@ -56,6 +58,7 @@
"Address",
"BindAddress",
"Network",
"Port",
"ICMPPort",
"TCPPort",
"UDPPort",
Expand All @@ -70,4 +73,5 @@
"MaintenanceStatus",
"ActiveStatus",
"UnknownStatus",
"Manager",
]
Loading

0 comments on commit 2894855

Please sign in to comment.