Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
67 changes: 34 additions & 33 deletions ops/_private/harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,46 +335,47 @@ def _event_context(self, event_name: str):

Example usage:

# this is how we test that attempting to write a remote app's
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is private, so only someone developing Harness would see the change.

# databag will raise RelationDataError.
>>> with harness._event_context('foo'):
>>> with pytest.raises(ops.model.RelationDataError):
>>> my_relation.data[remote_app]['foo'] = 'bar'

# this is how we test with 'realistic conditions' how an event handler behaves
# when we call it directly -- i.e. without going through harness.add_relation
>>> # this is how we test that attempting to write a remote app's
>>> # databag will raise RelationDataError.
>>> def test_foo():
>>> class MyCharm:
>>> ...
>>> def event_handler(self, event):
>>> # this is expected to raise an exception
>>> event.relation.data[event.relation.app]['foo'] = 'bar'
>>>
>>> harness = Harness(MyCharm)
>>> event = MagicMock()
>>> event.relation = harness.charm.model.relations[0]
>>>
>>> with harness._event_context('my_relation_joined'):
>>> with pytest.raises(ops.model.RelationDataError):
>>> harness.charm.event_handler(event)
... with harness._event_context('foo'):
... with pytest.raises(ops.model.RelationDataError):
... my_relation.data[remote_app]['foo'] = 'bar'

>>> # this is how we test with 'realistic conditions' how an event handler behaves
>>> # when we call it directly -- i.e. without going through harness.add_relation
>>> def test_foo():
... class MyCharm:
... ...
... def event_handler(self, event):
... # this is expected to raise an exception
... event.relation.data[event.relation.app]['foo'] = 'bar'
...
... harness = Harness(MyCharm)
... event = MagicMock()
... event.relation = harness.charm.model.relations[0]
...
... with harness._event_context('my_relation_joined'):
... with pytest.raises(ops.model.RelationDataError):
... harness.charm.event_handler(event)

If event_name == '', conversely, the Harness will believe that no hook
is running, allowing temporary unrestricted access to read/write a relation's
databags even from inside an event handler.

>>> def test_foo():
>>> class MyCharm:
>>> ...
>>> def event_handler(self, event):
>>> # this is expected to raise an exception since we're not leader
>>> event.relation.data[self.app]['foo'] = 'bar'
>>>
>>> harness = Harness(MyCharm)
>>> event = MagicMock()
>>> event.relation = harness.charm.model.relations[0]
>>>
>>> with harness._event_context('my_relation_joined'):
>>> harness.charm.event_handler(event)
... class MyCharm:
... ...
... def event_handler(self, event):
... # this is expected to raise an exception since we're not leader
... event.relation.data[self.app]['foo'] = 'bar'
...
... harness = Harness(MyCharm)
... event = MagicMock()
... event.relation = harness.charm.model.relations[0]
...
... with harness._event_context('my_relation_joined'):
... harness.charm.event_handler(event)

"""
return self._framework._event_context(event_name)
Expand Down
12 changes: 6 additions & 6 deletions ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,16 @@ def set_results(self, results: dict[str, Any]):
"""Report the result of the action.

Juju eventually only accepts a str:str mapping, so we will attempt
to flatten any more complex data structure like so::
to flatten any more complex data structure like so:

>>> {'a': 'b'} # becomes: 'a'='b'
>>> {'a': {'b': 'c'}} # becomes: 'a.b'='c'
>>> {'a': {'b': 'c', 'd': 'e'}} # becomes: 'a.b'='c', 'a.d' = 'e'
>>> {'a.b': 'c', 'a.d': 'e'} # equivalent to previous
* ``{'a': 'b'}`` becomes ``'a'='b'``
* ``{'a': {'b': 'c'}}`` becomes ``'a.b'='c'``
* ``{'a': {'b': 'c', 'd': 'e'}}`` becomes ``'a.b'='c', 'a.d' = 'e'``
* ``{'a.b': 'c', 'a.d': 'e'}`` is equivalent to the previous example

Note that duplicate keys are not allowed, so this is invalid::

>>> {'a': {'b': 'c'}, 'a.b': 'c'}
{'a': {'b': 'c'}, 'a.b': 'c'}

Note that the resulting keys must start and end with lowercase
alphanumeric, and can only contain lowercase alphanumeric, hyphens
Expand Down
10 changes: 8 additions & 2 deletions ops/framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -943,10 +943,16 @@ def _event_context(self, event_name: str):
is completed.

Usage:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is private, so only someone developing Ops would see it.

>>> import ops
>>> from ops import testing
>>> harness = testing.Harness(ops.CharmBase)
>>> with harness._event_context('db-relation-changed'):
>>> print('Harness thinks it is running an event hook.')
... # Harness thinks it is running an event hook.
... pass
>>> with harness._event_context(''):
>>> print('harness thinks it is not running an event hook.')
... # Harness thinks it is not running an event hook.
... pass
"""
backend: _ModelBackend | None = self.model._backend if self.model else None
if not backend:
Expand Down
4 changes: 4 additions & 0 deletions ops/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -2979,6 +2979,10 @@ def exec(
below, however, input/output handling is a bit more complex. Some
examples are shown below::

# These two lines are related to documentation testing, not exec()
>>> import pytest
>>> pytest.skip('these examples require a running Pebble')

# Simple command with no output; just check exit code
>>> process = client.exec(['send-emails'])
>>> process.wait()
Expand Down
8 changes: 6 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,16 @@ passenv =
PEBBLE
dependency_groups = unit, xdist
commands =
pytest -n auto \
pytest --doctest-modules -n auto \
--ignore={toxinidir}/docs \
--ignore={toxinidir}/release.py \
--ignore={[vars]tst_path}smoke \
--ignore={[vars]tst_path}integration \
--ignore={[vars]tst_path}benchmark \
--ignore={[vars]testing_tst_path}benchmark \
--ignore={[vars]tracing_tst_path} \
--ignore={[vars]examples_path} \
--ignore={[vars]tst_path}charms \
-v --tb native \
-W 'ignore:Harness is deprecated:PendingDeprecationWarning' {posargs}

Expand All @@ -110,7 +113,8 @@ passenv =
dependency_groups = unit, coverage
commands =
coverage run --source={[vars]src_path},{[vars]testing_src_path} \
--branch -m pytest --ignore={[vars]tst_path}smoke \
--branch -m pytest \
--ignore={[vars]tst_path}smoke \
Comment on lines -113 to +117
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I originally had the doctests running for coverage and then changed my mind on that. However, I think it is nicer to keep the ignores on separate lines.

--ignore={[vars]tst_path}integration \
--ignore={[vars]tst_path}benchmark \
--ignore={[vars]testing_tst_path}benchmark \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -950,15 +950,17 @@ def charm_tracing_config(
proceed with charm tracing (with or without tls, as appropriate)

Usage:
>>> from lib.charms.tempo_coordinator_k8s.v0.charm_tracing import trace_charm
>>> from lib.charms.tempo_coordinator_k8s.v0.tracing import charm_tracing_config
>>> @trace_charm(tracing_endpoint="my_endpoint", cert_path="cert_path")
>>> class MyCharm(...):
>>> _cert_path = "/path/to/cert/on/charm/container.crt"
>>> def __init__(self, ...):
>>> self.tracing = TracingEndpointRequirer(...)
>>> self.my_endpoint, self.cert_path = charm_tracing_config(
... self.tracing, self._cert_path)

from lib.charms.tempo_coordinator_k8s.v0.charm_tracing import trace_charm
from lib.charms.tempo_coordinator_k8s.v0.tracing import charm_tracing_config
@trace_charm(tracing_endpoint="my_endpoint", cert_path="cert_path")
class MyCharm(...):
_cert_path = "/path/to/cert/on/charm/container.crt"
def __init__(self, ...):
self.tracing = TracingEndpointRequirer(...)
self.my_endpoint, self.cert_path = charm_tracing_config(
self.tracing, self._cert_path
)
Comment on lines -953 to +963
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically, I should provide a clean for this upstream, but we're considering reimplementing this library rather than vendoring it, and there's the "move all the interface libraries" project as well, so it doesn't seem worth doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

My 2c:

  • let's not run doctests on vendored code
  • testing charm libs is outside of ops scope (albeit great for community)
  • doc-testing charm libs is even more so (albeit helps to keep charm lib doc strings in sync with charm lib code: chore: fix charm lib doc string haproxy-operator#163)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The challenge here is that pytest's doctest integration doesn't provide much in the way of configuration (pytest-doctestplus is better, but still doesn't have exactly what we want). There's no "ignore" option so to do this we'd have to have a lot of glob statements that match the files to be tested and don't match the files to not be tested.

"""
if not endpoint_requirer.is_ready():
return None, None
Expand Down