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

Add new testing function PatchConfiguration #76

Merged
merged 1 commit into from Apr 21, 2016

Conversation

ealter
Copy link
Contributor

@ealter ealter commented Apr 1, 2016

This creates a new way to mock out staticconf in tests. MockConfiguration only allows you to directly replace the entire namespace. However, PatchConfiguration patches the namespace preserving existing values. This allows you to mock out only a piece of the namespace, which can be useful for things like feature toggles.

"""

def setup(self):
self.old_values = dict(self.namespace.get_config_values())
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably want deep copies throughout this function

@ealter
Copy link
Contributor Author

ealter commented Apr 12, 2016

@dnephin what do you think of this branch (aside from @asottile's comments)

@dnephin
Copy link
Owner

dnephin commented Apr 12, 2016

I don't think I would ever use it myself. I prefer to wipe the old state entirely, so that I know exactly what I'm testing against, and not have my tests break unexpectedly if someone modifies the normal environment.

That said, I guess this is how mock.patch.dict() works, so I there is precedent, and I'd be fine accepting the PR (with the change asottile suggested)

@ealter
Copy link
Contributor Author

ealter commented Apr 12, 2016

We have a couple uses of this class already at Yelp (example: feature_toggles), so I thought I'd upstream it. I'll go address asottile's comments.

@ealter ealter force-pushed the eliot_update_configuration_testing branch from 62d37f7 to c4b0d38 Compare April 12, 2016 21:24
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 96.821% when pulling c4b0d38 on ealter:eliot_update_configuration_testing into 45a3d07 on dnephin:master.

@ealter
Copy link
Contributor Author

ealter commented Apr 21, 2016

Bump @dnephin any updates on this?

@dnephin dnephin merged commit 68192df into dnephin:master Apr 21, 2016
@ealter
Copy link
Contributor Author

ealter commented Apr 21, 2016

Thanks @dnephin can you also bump the version when you get a chance?

@ealter ealter deleted the eliot_update_configuration_testing branch April 21, 2016 20:02
@ealter ealter mentioned this pull request May 2, 2016
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.

None yet

4 participants