-
Notifications
You must be signed in to change notification settings - Fork 121
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
docs(testing): update code samples in ops.testing from unittest to pytest style #1157
docs(testing): update code samples in ops.testing from unittest to pytest style #1157
Conversation
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.
Looks good, thanks! A couple of questions about seemingly no-op indentation changes, and a suggestion about simplifying one of the examples.
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, I think this approach works nicely, thanks!
One small suggestion, and one place where I think the indentation isn't right.
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.
Thanks; looks good!
While working on another ticket, I noticed that there's also this unittest style doc in testing.py that should be updated as well. |
Yeah, it would be nice if we could just tweak the text there to be more general. Something like: "Always call |
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.
Let's just get that tweak to the cleanup
docstring in and then this is good to go!
Since we are encouraging charm tests to be written with pytest, we should use that in test code examples in ops.testing.
Only code samples from
ops/testing.py
are updated, other files/test cases are not updated.Closes #1154.