-
Notifications
You must be signed in to change notification settings - Fork 339
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
Make Django assertions available outside of a TestCase class #709
Conversation
This is my attempt at addressing some of the concerns in #232. |
cc3f8e8
to
80a72e1
Compare
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.
This is looking good from a quick glance - thanks for picking it up.
30523ea
to
2202bb1
Compare
Or replace sys.modules[__name__]
…On Sun, 16 Jun 2019, 10:43 Pascal Chambon, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pytest_django/asserts.py
<#709 (comment)>
:
> + getattr(test_case, name)(*args, **kwargs)
+
+ return assertion_func
+
+
+__all__ = []
+asserts = set()
+asserts.update(
+ set(attr for attr in vars(TestCase) if attr.startswith('assert')),
+ set(attr for attr in vars(SimpleTestCase) if attr.startswith('assert')),
+ set(attr for attr in vars(LiveServerTestCase) if attr.startswith('assert')),
+ set(attr for attr in vars(TransactionTestCase) if attr.startswith('assert')),
+)
+
+for assert_func in asserts:
+ locals()[assert_func] = _wrapper(assert_func)
locals() is not meant to be always settable, maybe juste use globals() to
make it clearer for newcomers?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#709?email_source=notifications&email_token=AADFATAOVYCMNRI6QL2HNKLP2YDNLA5CNFSM4G44C6M2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3VB3AI#pullrequestreview-250224001>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADFATDVJNANXY3Y5OJKR6LP2YDNLANCNFSM4G44C6MQ>
.
|
Approach looks straightforward and handy, thanks for this PR :) |
2202bb1
to
b2fa461
Compare
@blueyed Any thoughts on when you'd want to merge this? |
544cf4d
to
dcee2d6
Compare
A bug was discovered in the way that asserts as contextmanagers was being handled. The contextmanager asserts needed to return an actual class, and not just act solely as an assert. This commit fixes that bug
dcee2d6
to
3ae530b
Compare
Thanks for the update, I've pushed two improvements (better docs (star imports are discouraged), and changed it to use |
It looks so simple but so necessary. I look forward to the merge of this PR. |
@blueyed Thanks so much for those improvements! 🙏 |
Thanks! :) |
Released in 3.8.0.. 🎉 |
- api.test_serializers - api.test_views - documents.test_models - events.test_models - front.test_templatetags - front.test_views (partially) - lecturers.test_ajax - lectures.test_models Some files couldn't be rewritten because of missing assertion methods: pytest-dev/pytest-django#709 Given that this PR was recently merged, after upgrading dependencies we could take another shot at this.
@wgordon17 Any reason for not including |
@premchalmeti, since this repo focuses on Django, it made more sense to focus only on the Django asserts. Additionally, most (if not all) the |
No description provided.