(#1222) Fixed test methods sharing issue #1225
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1225 +/- ##
===========================================
- Coverage 89.22% 89.2% -0.03%
+ Complexity 1660 1659 -1
===========================================
Files 279 279
Lines 3992 3992
Branches 213 213
===========================================
- Hits 3562 3561 -1
Misses 396 396
- Partials 34 35 +1
Continue to review full report at Codecov.
|
|
Job #1225 is now in scope, role is |
|
This pull request #1225 is assigned to @victornoel/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job |
There was a problem hiding this comment.
@fanifieiev see my comments, I think it would be better to simply write things more procedurally in the tests to be sure you are only testing IteratorOf and not other things.
| @@ -60,7 +62,12 @@ public void emptyIteratorThrowsException() { | |||
|
|
|||
| @Test | |||
| public void nonEmptyIteratorDoesNotHaveNext() { | |||
| final IteratorOf<Integer> iterator = this.iteratorWithFetchedElements(); | |||
| final Iterator<Integer> iterator = new Skipped<>( | |||
There was a problem hiding this comment.
@fanifieiev please do not rely on Skipped here, it's too far away from the tested class to make the test robust
| @@ -70,9 +77,15 @@ public void nonEmptyIteratorDoesNotHaveNext() { | |||
|
|
|||
| @Test | |||
| public void nonEmptyIteratorThrowsException() { | |||
| final Iterator<Character> iterator = new Skipped<>( | |||
There was a problem hiding this comment.
@fanifieiev please do not rely on Skipped here, it's too far away from the tested class to make the test robust
There was a problem hiding this comment.
@victornoel Agree, made the fix
|
@0crat status |
|
@fanifieiev This is what I know about this job in C63314D6Z, as in §32:
|
|
@paulodamaso it's good to merge |
|
@victornoel Thanks |
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso Done! FYI, the full log is here (took me 12min) |
|
The job #1225 is now out of scope |
|
@sereshqua/z please review this job completed by @victornoel/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
|
Payment to |
|
@victornoel please make sure you will try to find at least 3 issues during next CR, thanks |
|
@sereshqua yes |
|
@0crat quality acceptable |
|
Quality review completed: +4 point(s) just awarded to @sereshqua/z |
|
Order was finished, quality is "acceptable": +15 point(s) just awarded to @victornoel/z |
This pull request is for issue 1222
According to the article https://www.yegor256.com/2016/05/03/test-methods-must-share-nothing.html
the test methods must not share any methods or data.