Skip to content

Commit

Permalink
feat!: use sets for the state components (#134)
Browse files Browse the repository at this point in the history 
* Remove _DCBase.

* Adjust the consistency checker and expose the Resource class.

* Finish the conversion (all tests pass).

* Don't add __eq__ for now.

* Update scenario/mocking.py

* Allow getting components by passing in the old entity.

* Revert back to the simpler get_ methods.

* Fix merges.

* Remove unused method (was used in the old binding, not generally useful).

* Add a basic test for resources.

* Add a basic test for resources.

* Make networks a set as well.
  • Loading branch information
@tonyandrewmeyer
tonyandrewmeyer committed Sep 2, 2024
1 parent 0c55e64 commit 5285060
Show file tree
Hide file tree
Showing 21 changed files with 451 additions and 283 deletions.
87 changes: 44 additions & 43 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,30 +322,29 @@ class MyCharm(ops.CharmBase):


def test_relation_data():
state_in = scenario.State(relations=[
scenario.Relation(
endpoint="foo",
interface="bar",
remote_app_name="remote",
local_unit_data={"abc": "foo"},
remote_app_data={"cde": "baz!"},
),
])
rel = scenario.Relation(
endpoint="foo",
interface="bar",
remote_app_name="remote",
local_unit_data={"abc": "foo"},
remote_app_data={"cde": "baz!"},
)
state_in = scenario.State(relations={rel})
ctx = scenario.Context(MyCharm, meta={"name": "foo"})

state_out = ctx.run(ctx.on.start(), state_in)

assert state_out.relations[0].local_unit_data == {"abc": "baz!"}
# you can do this to check that there are no other differences:
assert state_out.relations == [
assert state_out.get_relation(rel.id).local_unit_data == {"abc": "baz!"}
# You can do this to check that there are no other differences:
assert state_out.relations == {
scenario.Relation(
endpoint="foo",
interface="bar",
remote_app_name="remote",
local_unit_data={"abc": "baz!"},
remote_app_data={"cde": "baz!"},
),
]
}

# which is very idiomatic and superbly explicit. Noice.
```
Expand Down Expand Up @@ -381,11 +380,11 @@ be mindful when using `PeerRelation` not to include **"this unit"**'s ID in `pee
be flagged by the Consistency Checker:

```python
state_in = scenario.State(relations=[
state_in = scenario.State(relations={
scenario.PeerRelation(
endpoint="peers",
peers_data={1: {}, 2: {}, 42: {'foo': 'bar'}},
)])
)})

meta = {
"name": "invalid",
Expand Down Expand Up @@ -496,7 +495,7 @@ If you want to, you can override any of these relation or extra-binding associat

```python
state = scenario.State(networks={
'foo': scenario.Network.default(private_address='192.0.2.1')
scenario.Network.default("foo", private_address='192.0.2.1')
})
```

Expand All @@ -508,15 +507,15 @@ When testing a Kubernetes charm, you can mock container interactions. When using
be no containers. So if the charm were to `self.unit.containers`, it would get back an empty dict.

To give the charm access to some containers, you need to pass them to the input state, like so:
`State(containers=[...])`
`State(containers={...})`

An example of a state including some containers:

```python
state = scenario.State(containers=[
state = scenario.State(containers={
scenario.Container(name="foo", can_connect=True),
scenario.Container(name="bar", can_connect=False)
])
})
```

In this case, `self.unit.get_container('foo').can_connect()` would return `True`, while for 'bar' it would give `False`.
Expand All @@ -535,7 +534,7 @@ container = scenario.Container(
can_connect=True,
mounts={'local': scenario.Mount(location='/local/share/config.yaml', source=local_file)}
)
state = scenario.State(containers=[container])
state = scenario.State(containers={container})
```

In this case, if the charm were to:
Expand Down Expand Up @@ -572,8 +571,8 @@ def test_pebble_push():
can_connect=True,
mounts={'local': Mount(location='/local/share/config.yaml', source=local_file.name)}
)
state_in = scenario.State(containers=[container])
ctx = scenario.Context(
state_in = State(containers={container})
ctx = Context(
MyCharm,
meta={"name": "foo", "containers": {"foo": {}}}
)
Expand Down Expand Up @@ -606,7 +605,7 @@ class MyCharm(ops.CharmBase):

def test_pebble_push():
container = scenario.Container(name='foo', can_connect=True)
state_in = scenario.State(containers=[container])
state_in = scenario.State(containers={container})
ctx = scenario.Context(
MyCharm,
meta={"name": "foo", "containers": {"foo": {}}}
Expand Down Expand Up @@ -652,7 +651,7 @@ def test_pebble_exec():
stdout=LS_LL)
}
)
state_in = scenario.State(containers=[container])
state_in = scenario.State(containers={container})
ctx = scenario.Context(
MyCharm,
meta={"name": "foo", "containers": {"foo": {}}},
Expand Down Expand Up @@ -708,7 +707,7 @@ 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(storage=[storage])) as mgr:
with ctx.manager(ctx.on.update_status(), scenario.State(storages={storage})) as mgr:
foo = mgr.charm.model.storages["foo"][0]
loc = foo.location
path = loc / "myfile.txt"
Expand Down Expand Up @@ -753,11 +752,11 @@ So a natural follow-up Scenario test suite for this case would be:
ctx = scenario.Context(MyCharm, meta=MyCharm.META)
foo_0 = scenario.Storage('foo')
# The charm is notified that one of the storages it has requested is ready:
ctx.run(ctx.on.storage_attached(foo_0), scenario.State(storage=[foo_0]))
ctx.run(ctx.on.storage_attached(foo_0), scenario.State(storages={foo_0}))

foo_1 = scenario.Storage('foo')
# The charm is notified that the other storage is also ready:
ctx.run(ctx.on.storage_attached(foo_1), scenario.State(storage=[foo_0, foo_1]))
ctx.run(ctx.on.storage_attached(foo_1), scenario.State(storages={foo_0, foo_1}))
```

## Ports
Expand All @@ -766,7 +765,7 @@ Since `ops 2.6.0`, charms can invoke the `open-port`, `close-port`, and `opened-

- simulate a charm run with a port opened by some previous execution
ctx = scenario.Context(MyCharm, meta=MyCharm.META)
ctx.run(ctx.on.start(), scenario.State(opened_ports=[scenario.TCPPort(42)]))
ctx.run(ctx.on.start(), scenario.State(opened_ports={scenario.TCPPort(42)}))
```
- assert that a charm has called `open-port` or `close-port`:
```python
Expand All @@ -775,7 +774,7 @@ state1 = ctx.run(ctx.on.start(), scenario.State())
assert state1.opened_ports == [scenario.TCPPort(42)]
state2 = ctx.run(ctx.on.stop(), state1)
assert state2.opened_ports == []
assert state2.opened_ports == {}
```

## Secrets
Expand All @@ -784,12 +783,12 @@ Scenario has secrets. Here's how you use them.

```python
state = scenario.State(
secrets=[
secrets={
scenario.Secret(
{0: {'key': 'public'}},
id='foo',
)
]
),
},
)
```

Expand All @@ -813,30 +812,30 @@ To specify a secret owned by this unit (or app):

```python
state = scenario.State(
secrets=[
secrets={
scenario.Secret(
{0: {'key': 'private'}},
id='foo',
owner='unit', # or 'app'
remote_grants={0: {"remote"}}
# the secret owner has granted access to the "remote" app over some relation with ID 0
)
]
),
},
)
```

To specify a secret owned by some other application and give this unit (or app) access to it:

```python
state = scenario.State(
secrets=[
secrets={
scenario.Secret(
{0: {'key': 'public'}},
id='foo',
# owner=None, which is the default
revision=0, # the revision that this unit (or app) is currently tracking
)
]
),
},
)
```

Expand All @@ -853,15 +852,16 @@ class MyCharmType(ops.CharmBase):
assert self.my_stored_state.foo == 'bar' # this will pass!


state = scenario.State(stored_state=[
state = scenario.State(stored_states={
scenario.StoredState(
owner_path="MyCharmType",
name="my_stored_state",
content={
'foo': 'bar',
'baz': {42: 42},
})
])
}),
},
)
```

And the charm's runtime will see `self.my_stored_state.foo` and `.baz` as expected. Also, you can run assertions on it on
Expand All @@ -879,7 +879,8 @@ So, the only consistency-level check we enforce in Scenario when it comes to res
import pathlib

ctx = scenario.Context(MyCharm, meta={'name': 'juliette', "resources": {"foo": {"type": "oci-image"}}})
with ctx.manager(ctx.on.start(), scenario.State(resources={'foo': '/path/to/resource.tar'})) as mgr:
resource = scenario.Resource(name='foo', path='/path/to/resource.tar')
with ctx.manager(ctx.on.start(), scenario.State(resources={resource})) as mgr:
# 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')
assert path == pathlib.Path('/path/to/resource.tar')
Expand Down Expand Up @@ -1060,7 +1061,7 @@ class MyCharm(ops.CharmBase):
def test_start_on_deferred_update_status(MyCharm):
foo_relation = scenario.Relation('foo')
scenario.State(
relations=[foo_relation],
relations={foo_relation},
deferred=[
scenario.deferred('foo_relation_changed',
handler=MyCharm._on_foo_relation_changed,
Expand Down
2 changes: 2 additions & 0 deletions scenario/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
Notice,
PeerRelation,
Relation,
Resource,
Secret,
State,
StateValidationError,
Expand Down Expand Up @@ -52,6 +53,7 @@
"ICMPPort",
"TCPPort",
"UDPPort",
"Resource",
"Storage",
"StoredState",
"State",
Expand Down
29 changes: 11 additions & 18 deletions scenario/consistency_checker.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import marshal
import os
import re
from collections import Counter, defaultdict
from collections import defaultdict
from collections.abc import Sequence
from numbers import Number
from typing import TYPE_CHECKING, Iterable, List, NamedTuple, Tuple, Union
Expand Down Expand Up @@ -108,7 +108,7 @@ def check_resource_consistency(
warnings = []

resources_from_meta = set(charm_spec.meta.get("resources", {}))
resources_from_state = set(state.resources)
resources_from_state = {resource.name for resource in state.resources}
if not resources_from_meta.issuperset(resources_from_state):
errors.append(
f"any and all resources passed to State.resources need to have been defined in "
Expand Down Expand Up @@ -265,7 +265,7 @@ def _check_storage_event(
f"storage event {event.name} refers to storage {storage.name} "
f"which is not declared in the charm metadata (metadata.yaml) under 'storage'.",
)
elif storage not in state.storage:
elif storage not in state.storages:
errors.append(
f"cannot emit {event.name} because storage {storage.name} "
f"is not in the state.",
Expand Down Expand Up @@ -330,11 +330,11 @@ def check_storages_consistency(
**_kwargs, # noqa: U101
) -> Results:
"""Check the consistency of the state.storages with the charm_spec.metadata (metadata.yaml)."""
state_storage = state.storage
state_storage = state.storages
meta_storage = (charm_spec.meta or {}).get("storage", {})
errors = []

if missing := {s.name for s in state.storage}.difference(
if missing := {s.name for s in state_storage}.difference(
set(meta_storage.keys()),
):
errors.append(
Expand All @@ -347,7 +347,7 @@ def check_storages_consistency(
if tag in seen:
errors.append(
f"duplicate storage in State: storage {s.name} with index {s.index} "
f"occurs multiple times in State.storage.",
f"occurs multiple times in State.storages.",
)
seen.append(tag)

Expand Down Expand Up @@ -465,10 +465,8 @@ def check_network_consistency(
if metadata.get("scope") != "container" # mark of a sub
}

state_bindings = set(state.networks)
if diff := state_bindings.difference(
meta_bindings.union(non_sub_relations).union(implicit_bindings),
):
state_bindings = {network.binding_name for network in state.networks}
if diff := state_bindings.difference(meta_bindings.union(non_sub_relations)):
errors.append(
f"Some network bindings defined in State are not in metadata.yaml: {diff}.",
)
Expand Down Expand Up @@ -598,11 +596,6 @@ def check_containers_consistency(
f"Missing from metadata: {diff}.",
)

# guard against duplicate container names
names = Counter(state_containers)
if dupes := [n for n in names if names[n] > 1]:
errors.append(f"Duplicate container name(s): {dupes}.")

return Results(errors, [])


Expand Down Expand Up @@ -633,12 +626,12 @@ def check_storedstate_consistency(
state: "State",
**_kwargs, # noqa: U101
) -> Results:
"""Check the internal consistency of `state.storedstate`."""
"""Check the internal consistency of `state.stored_states`."""
errors = []

# Attribute names must be unique on each object.
names = defaultdict(list)
for ss in state.stored_state:
for ss in state.stored_states:
names[ss.owner_path].append(ss.name)
for owner, owner_names in names.items():
if len(owner_names) != len(set(owner_names)):
Expand All @@ -647,7 +640,7 @@ def check_storedstate_consistency(
)

# The content must be marshallable.
for ss in state.stored_state:
for ss in state.stored_states:
# We don't need the marshalled state, just to know that it can be.
# This is the same "only simple types" check that ops does.
try:
Expand Down
Loading

0 comments on commit 5285060

Please sign in to comment.