refactor test in the common package#13
refactor test in the common package#13ignatmaloukhov wants to merge 7 commits intoeditorconfig:masterfrom
Conversation
There was a problem hiding this comment.
Thanks for the PR.
The only thing that I think I would want to ask to change is to keep using assertj for assertions. From my opinion, the assertj api is more clean in terms of what is expected, and what is actual argument, and is more clear in assertion failure messages.
Also, do you mind taking a look into the other test classes as well?
|
Closes #11 |
Thanks for the review. This was the first try to get your opinion on the proposed changes. I am ready to complete all the test classes of course. I returned |
Great! Thanks. Please, ping me here when the remaining tests will be ready. |
| @ParameterizedTest | ||
| @EnumSource(value = EndOfLine.class, names = "CARRIAGE_RERUN_LINE_FEED", mode = Mode.EXCLUDE) | ||
| void isSingleCharacter_true(EndOfLine source) { | ||
| void isSingleCharacter_СrLfEof_True(EndOfLine source) { |
There was a problem hiding this comment.
It is everything except the CRLF, which is not a single character, so the name of the test is misleading :)
mipo256
left a comment
mipo256
left a comment
There was a problem hiding this comment.
In general, everything is alright, The only thing I would suggest is to separate the pom.xml changes from the refactoring in tests into two separate commits
|
Hey @ignatmaloukhov! Are you on track? Shall I take over if you're limited on time? |
3 participants
Refactored tests in the common package