Fix test helpers for results#7002
Conversation
|
This change breaks a lot. Is there an issue that explains what is happening here, or why it needs doing? Or can this be closed? |
bpoldrack
force-pushed
the
bf-result-assertions
branch
2 times, most recently
from
b52f771 to
9e23c57
Compare
Codecov ReportBase: 74.88% // Head: 75.61% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## maint #7002 +/- ##
==========================================
+ Coverage 74.88% 75.61% +0.73%
==========================================
Files 355 355
Lines 59105 59312 +207
Branches 6318 6320 +2
==========================================
+ Hits 44262 44851 +589
+ Misses 14829 14446 -383
- Partials 14 15 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
bpoldrack
force-pushed
the
bf-result-assertions
branch
from
9e23c57 to
ccb7f7a
Compare
|
Code Climate has analyzed commit 16a162d and detected 0 issues on this pull request. View more on Code Climate. |
| ### Tests | ||
|
|
||
| - Fix broken test helpers for result record testing that would falsely pass. | ||
| https://github.com/datalad/datalad/pull/7002 (by @bpoldrack) |
|
why |
|
nice find -- thanks @bpoldrack ! IMHO should go against |
The helpers for testing result records had a couple of bugs. Most noteably, some (e.g. `assert_status`) would pass when no result was given at all. Therefore command calls that did not return any result were passing assertions about the `status` field of their results. Furthermore, most of those helpers did not properly deal with all three "representations" of results that are (and need to be) passed to them: 1. list of dict 2. single dict 3. generator That's because `ensure_list` is used with the incoming results, but this turns dicts that are not encapsulated by a list into a list of their keys rather than a single-item list of dict. Reason for that is, that a dict is iterable. This patch should fix this.
Previous commit fixed a couple of test helpers for result record testing. Those were falsely passing when they shouldn't and hid bugs/mistakes in the tests themselves. Most noteably, that's command calls (mostly `clone`) failing to set `result_xfm` correctly and therefore not getting result records back, but then proceeding to assertions about such result records.
bpoldrack
force-pushed
the
bf-result-assertions
branch
from
16a162d to
4ce17b1
Compare
|
Thx, @yarikoptic. All addressed, I think. |
|
Looks great to me! -deprecated -- seems to be a real one to fix up, filed datalad/datalad-deprecated#69 . There is some minor nit-picking I could have done (avoidable diff now that there is a helper etc) but to progress -- let's just merge! Thank you @bpoldrack -- I will submit a test PR for merge maint into master shortly after this merge. Hopefully we would not get many failures in other untested here extensions whenever we release datalad ;-) |
|
PR released in |
4 participants
The helpers for testing result records had a couple of bugs.
Most noteably, some (e.g.
assert_status) would pass when no result wasgiven at all. Therefore command calls that did not return any result
were passing assertions about the
statusfield of their results.Furthermore, most of those helpers did not properly deal with all three
"representations" of results that are (and need to be) passed to them:
That's because
ensure_listis used with the incoming results, but thisturns dicts that are not encapsulated by a list into a list of their
keys rather than a single-item list of dict. Reason for that is, that a
dict is iterable.
The falsely passing helpers also hid bugs in the tests - mostly command calls (
clone) failing to setresult_xfmcorrectly and therefore not getting result records back, but then proceeding to assertions about such result records.This patch should fix this.