Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: use sets for the state components #134

Merged
merged 12 commits into from
Jul 9, 2024
Merged

feat!: use sets for the state components #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d880bae
Remove _DCBase.
tonyandrewmeyer Apr 4, 2024
1f18d0d
Adjust the consistency checker and expose the Resource class.
tonyandrewmeyer May 28, 2024
7c66562
Finish the conversion (all tests pass).
tonyandrewmeyer May 30, 2024
97422c6
Don't add __eq__ for now.
tonyandrewmeyer May 30, 2024
7e1bbc2
Update scenario/mocking.py
tonyandrewmeyer May 30, 2024
3b34ee3
Allow getting components by passing in the old entity.
tonyandrewmeyer May 31, 2024
9a6a18a
Revert back to the simpler get_ methods.
tonyandrewmeyer Jun 5, 2024
29bf3e5
Fix merges.
tonyandrewmeyer Jul 8, 2024
c2ff882
Remove unused method (was used in the old binding, not generally usef…
tonyandrewmeyer Jul 8, 2024
28b8765
Add a basic test for resources.
tonyandrewmeyer Jul 8, 2024
ffa3b7a
Add a basic test for resources.
tonyandrewmeyer Jul 8, 2024
f02c971
Make networks a set as well.
tonyandrewmeyer Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 44 additions & 43 deletions README.md
View file
Open in desktop
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 @@ -567,12 +566,12 @@ class MyCharm(ops.CharmBase):

def test_pebble_push():
with tempfile.NamedTemporaryFile() as local_file:
container = scenario,Container(
container = scenario.Container(
name='foo',
can_connect=True,
mounts={'local': Mount(location='/local/share/config.yaml', source=local_file.name)}
)
state_in = State(containers=[container])
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
Open in desktop
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
25 changes: 10 additions & 15 deletions scenario/consistency_checker.py
View file
Open in desktop
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 @@ -462,7 +462,7 @@ def check_network_consistency(
if metadata.get("scope") != "container" # mark of a sub
}

state_bindings = set(state.networks)
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 @@ -593,11 +593,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 @@ -628,12 +623,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 @@ -642,7 +637,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
Loading