-
Notifications
You must be signed in to change notification settings - Fork 68
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
Refactor settings to make override_settings in tests more consistent #168
Refactor settings to make override_settings in tests more consistent #168
Conversation
6c240da
to
a8d4fa0
Compare
cms_helper.py
Outdated
META_USE_SITES=True, | ||
META_USE_OG_PROPERTIES=True, | ||
META_USE_TWITTER_PROPERTIES=True, | ||
META_USE_SCHEMAORG_PROPERTIES=True, |
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.
these are meant to be used when running app_helper.py server
to run the sample project
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.
Oh, ok, I see... if I leave them there, some tests are failing somehow because I think tests use those settings.
For instance, test_meta.py::MetaObjectTestCase::test_defaults
fails because self.assertEqual(m.use_og, False)
is actually True
, because of META_USE_OG_PROPERTIES=True
in cms_helper.py
What's the best approach here? Override those settings globally in every test class, in order to reflect the actual default defined in settings.py
?
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.
I see
I can't think of a clean solution, so maybe the cleanest is to use override_settings
on BaseTestClass
to ensure all the tests use the desired setting no matter what cms_helper
sets?
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.
Done!
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #168 +/- ##
===========================================
+ Coverage 97.60% 97.62% +0.01%
===========================================
Files 8 8
Lines 460 463 +3
Branches 78 79 +1
===========================================
+ Hits 449 452 +3
Misses 8 8
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Description
Refactor settings to make override_settings in tests more consistent
References
Fix #167
Checklist
inv lint
changes
file included (see docs)