-
Notifications
You must be signed in to change notification settings - Fork 119
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.
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
fix(testing): reset collected statuses in Harness.evaluate_status #1048
Conversation
self.assertEqual(harness.model.app.status, ops.BlockedStatus('blocked app')) | ||
self.assertEqual(harness.model.unit.status, ops.BlockedStatus('blocked unit')) | ||
|
||
harness.charm.app_status_to_add = ops.ActiveStatus('active app') | ||
harness.charm.unit_status_to_add = ops.ActiveStatus('active unit') | ||
harness.evaluate_status() | ||
self.assertEqual(harness.model.app.status, ops.ActiveStatus('active app')) | ||
self.assertEqual(harness.model.unit.status, ops.ActiveStatus('active unit')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So evaluate_status calculates, commit somewhere else, and clears the collection for the next add_status?
Then should probably document that cannot do "additive tests" with evaluate_status.
Not sure which implementation is less confusing though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I've updated the evaluate_status
docstring to clarify that. I don't think "additive tests" is useful for this: before triggering the collect-status events, the statuses added should be reset.
ops/testing.py
Outdated
if self._backend._is_leader: | ||
self.charm.app._collected_statuses = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The distinction for leader seems inconsistent with the clear
:
If evaluate_status
clears the stage for the next add_status
, it's a harness-level function. Leadership status is on a different level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good call. I've now cleared this independent of is_leader
.
Currently you can't really call
Harness.evaluate_status()
twice in one test because it keeps the previous collected statuses around. Reset them before evaluating.I believe this addresses #736 (comment)