[Console] increased code coverage of Output classes #21124
Conversation
| { | ||
| $output = new ConsoleOutput(); | ||
| $output->setVerbosity(1); | ||
| $this->assertSame(1, $output->getVerbosity()); |
There was a problem hiding this comment.
Not sure if you should use the constant instead, it might break if the constant value is changed
There was a problem hiding this comment.
in this case it shouldn't matter, because the verbosity should be exactly the one set using the setter method
There was a problem hiding this comment.
Well it does matter, because if the constant value would get changed to a string, lets say 'very verbose', 1 would not match anymore.
There was a problem hiding this comment.
yes but here at unit test level we should only ensure that whatever value set by the setter is also returned by the getter, right?
There was a problem hiding this comment.
In this particular case you could see it like an Enum, but php doesn't support Enums sadly.
You could probably compare it to:
public function setVerbosity(OutputVerbosityEnum $level);When working with enum/constants, you should forget about the value and think in constant references only to prevent bugs in future versions.
There was a problem hiding this comment.
allright, changed 👍
|
LGTM too Status: Reviewed |
|
Thank you @ShinDarth. |
…nDarth) This PR was squashed before being merged into the 2.7 branch (closes #21124). Discussion ---------- [Console] increased code coverage of Output classes | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT This PR increases the coverage of Output classes of the Console component from 80.81% to 94.95% Commits ------- ab4ba23 [Console] increased code coverage of Output classes
This PR increases the coverage of Output classes of the Console component from 80.81% to 94.95%