Expand the testing module a little bit #230
Conversation
pypingou
commented
Oct 28, 2020
pypingou
commented
- Allow to use the re-written assert statement from pytest which provides a much more detailed output of where the assertion failed
- Check the topic and body separately before checking the entire objects.
pypingou
force-pushed
the
master
branch
2 times, most recently
from
47da851
to
0a6f4dc
Compare
|
Only the py27 tests are failing, one of the dependency seems to require 3.5 minimum apparently |
|
Sure can do |
pytest re-writes the assert method to be able to provide a much more
detailed output to the user explaining why a certain assertion failed.
This is really handy when debugging failing tests.
With this commits, we are telling pytest to rewrite the assert used
in the fedora_messaging.testing module so it returns more information
to the user.
Without this commit the error message looks like:
````
> assert msg == expected
E AssertionError
````
With this change, the error looks like:
````
E AssertionError: assert PullRequestFlagAddedV1(id='1a07142d-f730-41ef-b1ee-2881b37c7b03', topic='pagure.pull-request.flag.added', body={'pullr...updated': '1604049807', 'user': {'name': 'pingou', 'fullname': 'PY C', 'url_path': 'user/pingou'}}, 'agent': 'pingou'}) == PullRequestFlagAddedV1(id='4a6a4d00-937d-4e7f-9f6d-192c29d5aad9', topic='pagure.pull-request.flag.added', body={'pullr... 'date_updated': <ANY>, 'user': {'name': 'pingou', 'fullname': 'PY C', 'url_path': 'user/pingou'}}, 'agent': 'pingou'})
````
Adding the -v or -vv flag to pytest will increate the proportion of
the messages shown, but will not clearly show the difference between
the messages.
Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>
This makes it easier to debug what is failing in a test suite.
Was it the topic? The dict sent? Or something else in the object?
This together with the previous commit allowing to use the re-written
assertion from pytest makes it much easier to find out why a specific
assertion failed.
Without this commit the error message looks like:
````
> assert msg == expected
E AssertionError: assert PullRequestFlagAddedV1(id='1a07142d-f730-41ef-b1ee-2881b37c7b03', topic='pagure.pull-request.flag.added', body={'pullr...updated': '1604049807', 'user': {'name': 'pingou', 'fullname': 'PY C', 'url_path': 'user/pingou'}}, 'agent': 'pingou'}) == PullRequestFlagAddedV1(id='4a6a4d00-937d-4e7f-9f6d-192c29d5aad9', topic='pagure.pull-request.flag.added', body={'pullr... 'date_updated': <ANY>, 'user': {'name': 'pingou', 'fullname': 'PY C', 'url_path': 'user/pingou'}}, 'agent': 'pingou'})
````
Adding the -v or -vv flag will only show more of the messages without
pointing out where the messages differ.
With this change, the error looks like:
````
> assert msg.body == expected.body
E AssertionError: assert {'agent': 'pi...nknown', ...}} == {'agent': 'pin...nknown', ...}}
E Omitting 2 identical items, use -vv to show
E Differing items:
E {'pullrequest': {'assignee': None, 'branch': 'master', 'branch_from': 'master', 'cached_merge_status': 'unknown', ...}} != {'pullrequest': {'assignee': None, 'branch': 'master', 'branch_from': 'master', 'cached_merge_status': 'unknown', ...}}
E Use -v to get the full diff
````
And with the -v option passed to pytest:
````
> assert msg.body == expected.body
E AssertionError: assert {'agent': 'pi...nknown', ...}} == {'agent': 'pin...nknown', ...}}
E Common items:
[...]
E 'url_path': 'test',
E - 'user': {'fullname': 'PY C',
E + 'user': {'fullname': 'PY C', 'name': 'pingou'}},
E ? ++++++++++++++++++++
E - 'name': 'pingou',
E - 'url_path': 'user/pingou'}},
[...]
```
Ie: the dict are showned and diffed instead of the objects which makes
pytest point out exactly where the messages differ.
Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>
|
I've added an example before/after in each commit. |
I'd agree but they don't seem related to this PR (and in fact they don't seem related to fedora-messaging), from what I saw, one of the dependency of fedora-messaging is now py3 only. So unless we want to add an upper limit to that dependency, I'm not sure how we could fix it. Also should this be done in this PR or in another? |
The only circumstance under which a change should be merged without a full test suite pass is a critical security fix, and even then it's probably better to make sure the code actually works, so the Python 2 requirement either needs to be dropped or fixed. It should be fixable by explicitly defining pinned dependencies for Python 2 environments in https://github.com/fedora-infra/fedora-messaging/blob/stable/tox.ini. I don't have an opinion about it being a separate PR vs an introductory commit to this one. |
From 0.16 onward pyrsistent is no longer py2 compatible (requires 3.5+) and thus that breaks the test suite that still runs py2.7 for some of our older systems. Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>
Codecov Report
@@ Coverage Diff @@
## stable #230 +/- ##
=======================================
Coverage 95.68% 95.69%
=======================================
Files 13 13
Lines 1344 1346 +2
Branches 166 166
=======================================
+ Hits 1286 1288 +2
Misses 43 43
Partials 15 15
Continue to review full report at Codecov.
|
