#485 Introduce AbstractHmBody matcher and implementations

[t-izbassar]

PR for #485

[0crat]

@yegor256/z please, pay attention to this pull request

[0crat]

Job #785 is now in scope, role is REV

[codecov-io]

Codecov Report

Merging #785 into master will increase coverage by 0.1%.
The diff coverage is 85.71%.

@@             Coverage Diff             @@
##             master     #785     +/-   ##
===========================================
+ Coverage     76.45%   76.55%   +0.1%     
- Complexity      934      953     +19     
===========================================
  Files           212      215      +3     
  Lines          4447     4505     +58     
  Branches        353      358      +5     
===========================================
+ Hits           3400     3449     +49     
- Misses          898      904      +6     
- Partials        149      152      +3

Impacted Files	Coverage Δ	Complexity Δ
.../main/java/org/takes/facets/hamcrest/HmRsBody.java	100% <100%> (ø)	5 <5> (?)
.../main/java/org/takes/facets/hamcrest/HmRqBody.java	100% <100%> (ø)	5 <5> (?)
...java/org/takes/facets/hamcrest/AbstractHmBody.java	75% <75%> (ø)	5 <5> (?)
src/main/java/org/takes/facets/flash/RsFlash.java	84.21% <0%> (-11.25%)	11% <0%> (+4%)
src/main/java/org/takes/http/MainRemote.java	53.33% <0%> (-1.67%)	6% <0%> (-1%)
src/main/java/org/takes/rs/RsSimple.java	80% <0%> (+30%)	2% <0%> (+1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa0f212...8b90286. Read the comment docs.

[t-izbassar]

I've checked coverage lines. Jacoco qualify constructors in HmRqBody and in HmRsBody as not covered, that's why those classes are covered only for 33%. However, in AbstractHmBody coverage rate is 100% (for constructors) and as we only delegating construction to parent class I think that's enough. I don't want to write useless tests to cover all possible constructors.

Also there is describeTo which is not covered intentionally as implementation is not full. If you think that I should write test anyway I can do it.

[t-izbassar]

I was able to fix coverage report. Looks OK now. @yegor256 don't you mind finding someone to review this PR? Thanks.

[0crat]

This pull request #785 is assigned to @g4s8/z. The budget is 15 minutes, see §4. Please, read §27 and when you decide to accept the changes, inform @yegor256/z (the architect) right in this ticket. If you decide that this PR should not be accepted ever, also inform the architect.

[g4s8]

@t-izbassar I'm not sure that this PR resolves bug-reporter issue fully, as I understand he wanted to provide hamcrest matchers as constructor parameters (not string or byte-array values), like:

MatcherAssert.assertThat(
  response,
  new HmRsBody<>(Matchers.startsWith("<html"))
);

@carlosmiranda could you please take a look?

[t-izbassar]

@g4s8 I understand that. However body returns an inputStream from what we can read at most bytes. And at most we can only check that they are equal or maybe they starts with same sequence of bytes. I’m not sure that containing exact sequence of bytes is useful. That’s why I left the puzzle to distinguish bytes only bodies with texts. Your example will work only for text bodies.

[t-izbassar]

@g4s8 and how you think your example should work? I will read bytes from inputStream then I need to transform that bytes to a string and only then I can apply given matcher. That’s seems to me will lead to a wrong design with having to check if matcher is for certain type. However I can think of something if @carlosmiranda will suggests other possibilities.

[t-izbassar]

Hey @g4s8 what do you think? This puzzle with splitting for two different types of Bodies should be enough, so that we will have HmTextBody and HmBytesBody and we can safely apply matcher from your example to HmTextBody as it is body with string content. I think having them separate is better way than having to convert bytes to string if we have string matcher.

Or how we can convert string matcher to bytes matches? I think this will add complexity to the solution.

[g4s8]

@t-izbassar yes, I think it would be better. You can update a @todo (@todo #485:30min Right now we can only check that InputStream) with your suggestion and I'll accept this PR.

[g4s8]

@yegor256 I accept

[t-izbassar]

@yegor256 ping

[t-izbassar]

@yegor256 ping

[yegor256]

@rultor merge

[rultor]

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

[0crat]

@g4s8/z this job was assigned to you 5 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

[rultor]

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 17min)

[0crat]

Order was successfully finished: +15 points just awarded to @g4s8/z, total is +3790

[0crat]

Payment to ARC for a closed pull request, as in §28: +15 points just awarded to @yegor256/z, total is +9495

[0crat]

The job #785 is now out of scope