MultilineCommentOpeningClosingFixer - fix handling empty comment #4181
Conversation
|
Ping @Slamdunk as the author of fixer. |
| @@ -37,7 +37,9 @@ public function testFix($expected, $input = null) | |||
| public function provideDocblocksCases() | |||
| { | |||
| return [ | |||
| ['<?php /* Opening comment */'], | |||
There was a problem hiding this comment.
This is already tested below with '<?php /*** Opening Multiline comment */'
There was a problem hiding this comment.
I wanted implicit test as we change the comment with RegExes.
There was a problem hiding this comment.
I'm sorry I don't get your comment 😕
There was a problem hiding this comment.
When I started checking tests I wondered why the first one is for doc comment and there is none for comment - I (hopefully I'm not the only one) read tests from top to the bottom to understand what is going and and I prefer to have the simplest - happy paths - at the begging - that's why I've added this one and /**/.
There was a problem hiding this comment.
Ah ok. In this case it should be better to rearrange and rename the current test cases instead of repeating an existing one, IMHO
There was a problem hiding this comment.
@Slamdunk rearranged
| ['<?php /** Opening DocBlock */'], | ||
| ['<?php /**/'], |
There was a problem hiding this comment.
This is already tested below in '<?php /***/'
There was a problem hiding this comment.
You mean the expected value is checked to not being changes again, right? Wouldn't this test case make it more explicit?
There was a problem hiding this comment.
@keradus set up that all the expected values are also tested against themselves, this is done for every test-case in this library, so we can skip a lot of duplicated code in the tests
There was a problem hiding this comment.
That's how tricky I am, ain't I?
|
Thank you @kubawerlos. |
kubawerlos
deleted the
fix-multiline-comment-opening-closing-empty-comment
branch
Fixes #4179