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.

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

tests: remove unnecessary patching of private ops class #330

Open
wants to merge 2 commits into
base: 6/edge
Choose a base branch
from

Conversation

tonyandrewmeyer
Copy link

The tests currently patch ops.testing._TestingModelBackend, which is a private class. This will break in an upcoming release of ops.

The patching is actually not required any more, so can just be entirely removed rather than updated.

MiaAltieri
MiaAltieri previously approved these changes Sep 12, 2024
Copy link
Contributor

@MiaAltieri MiaAltieri left a comment

Choose a reason for hiding this comment

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

Thank you so much for this change @tonyandrewmeyer - Really appreciate your foresight here, I notified the DP team + I've made GitHub issues in the other Mongo* repos as to not forget about it

@phvalguima phvalguima self-requested a review September 13, 2024 08:15
Copy link

@phvalguima phvalguima left a comment

Choose a reason for hiding this comment

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

Hi @tonyandrewmeyer really appreciate your attention here. Thanks for highlighting the new changes in ops and how it will affect the team and the charms!

Left just a question, as some charms' tests do depend on a specific return from a mocked network_get. Can you just explain how should it work from now on?

@@ -1044,7 +1041,6 @@ def test_set_password_provided(self, connection):
# verify app data is updated and results are reported to user
self.assertEqual("canonical123", new_password)

@patch_network_get(private_address="1.1.1.1")

Choose a reason for hiding this comment

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

Some of our charms are using this specific value 1.1.1.1 to do further checks, e.g. here.

How can we now mock the network_get and return a specific private_address?

@tonyandrewmeyer
Copy link
Author

Left just a question, as some charms' tests do depend on a specific return from a mocked network_get. Can you just explain how should it work from now on?

When adding a relation, if there's no global network configured, then Harness checks for one specific to the (relation name, relation ID) and will add 10.0.0.10 if one doesn't exist, and checks for one specific to just the relation name and will add 192.0.2.0 if one doesn't exist.

If you need different addresses, then you can just call Harness.add_network, making sure you do this before adding the relation.

Calling with just the address (like harness.add_network('192.0.2.1')) sets up the default binding, or you can call with the endpoint (relation name) and relation_id set if it should be specific to those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants