AbstractHmBody doesn't allow to use different matchers for text bodies #813
Conversation
|
Job #813 is now in scope, role is |
Codecov Report
@@ Coverage Diff @@
## master #813 +/- ##
============================================
+ Coverage 76.54% 76.87% +0.32%
- Complexity 956 971 +15
============================================
Files 215 219 +4
Lines 4567 4610 +43
Branches 366 370 +4
============================================
+ Hits 3496 3544 +48
+ Misses 918 910 -8
- Partials 153 156 +3
Continue to review full report at Codecov.
|
|
@g4s8 ping |
| * @param expected String to test against | ||
| */ | ||
| public HmRqTextBody(final String expected) { | ||
| this(Matchers.containsString(expected)); |
There was a problem hiding this comment.
@t-izbassar I think it's not clear, because default matcher in Hamcrest library usually is Matchers.equalTo(), so as a user of HmRqTextBody I'd expect that body is equal to constructor parameter string
|
|
||
| @Override | ||
| public final void describeTo(final Description description) { | ||
| description.appendDescriptionOf(this.body); |
There was a problem hiding this comment.
@t-izbassar it will be not really informative, it should include not only expected result, but also actual value and some text description. Please see how it is implemented in AbstractHmHeader
There was a problem hiding this comment.
@g4s8 updated puzzle description to cover that issue.
| public void testsBodyValueContainsText() { | ||
| MatcherAssert.assertThat( | ||
| new RsWithBody("<html><h1>Hello</h1></html>"), | ||
| new HmRsTextBody("<h1>Hello</h1>") |
There was a problem hiding this comment.
@t-izbassar this test is not so obvious, please look at previous comment about default matcher in constructor
|
@g4s8 done. |
|
@yegor256 I accept |
|
@yegor256 can you merge this one? |
|
@rultor merge |
|
@elenavolokhova/z please review this job, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
|
@0crat quality good |
|
@elenavolokhova You can't do that, unless you have one of these roles: |
|
|
|
@elenavolokhova The job #813 is now out of scope |
Fix for #794.
I've added
AbstractHmTextBodymatcher, that converts body contents toStringand use that to match the contents with provided matcher. Added text body matchers forRequestandResponsewith relevant test cases.However, there still not covered descriptions of the fails and
describeMismatchSafelyis not implemented. Left puzzle to address this issue.This PR partially solves original issue. Left puzzle to address what was left (converting
AbstractHmBodytoAbstractHmBytesBody) as those are beyond 30 mins mark.