Skip to content

Commit

Permalink
fix: avoid changing os.environ in Harness (#1359)
Browse files Browse the repository at this point in the history
Harness needs to provide a `JUJU_VERSION` value to set up the
`_JujuContext`, but it doesn't have to be in `os.environ`, it can just
be in the dictionary passed to create the `_JujuContext` object.

In production, `os.environ` would actually have this, so it's more
realistic for it to be present, but it would also have lots of the other
`JUJU_` environment variables as well, and we don't want to have Harness
simulate all those in the environment - we want people working with the
ops tools to access those, not the environment directly.

This change [broke the tests of at least one
charm](canonical/charm-simple-streams#22)
because it patches the environment to have specific values, and then
creating the `Harness` object changes that. It seems better for us to
not do this - if we did want to populate the environment to mimic Juju
then we'd likely want that to be explicit, or done around the event
emitting. It was also an accidental backwards compatiblity break.
  • Loading branch information
tonyandrewmeyer committed Sep 5, 2024
1 parent a574456 commit e302b63
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,10 @@ def __init__(
actions: Optional[YAMLStringOrFile] = None,
config: Optional[YAMLStringOrFile] = None,
):
if 'JUJU_VERSION' not in os.environ:
os.environ['JUJU_VERSION'] = '0.0.0'
self._juju_context = _JujuContext.from_dict(os.environ)
context_environ = os.environ.copy()
if 'JUJU_VERSION' not in context_environ:
context_environ['JUJU_VERSION'] = '0.0.0'
self._juju_context = _JujuContext.from_dict(context_environ)
self._charm_cls = charm_cls
self._charm: Optional[CharmType] = None
self._charm_dir = 'no-disk-path' # this may be updated by _create_meta
Expand Down

0 comments on commit e302b63

Please sign in to comment.