[TASK] Add some tests for OutputFormatter (part 1)
#943
Conversation
oliverklee
added this to the
9.0.0: Drop support for PHP < 7.2 and clean up things milestone
oliverklee
changed the title
OutputFormatterOutputFormatter (part 1)
oliverklee
force-pushed
the
task/tests/output-formatter
branch
from
d06d72b to
98fe7b4
Compare
oliverklee
force-pushed
the
task/tests/output-formatter
branch
2 times, most recently
from
05beffb to
33db0fa
Compare
There was a problem hiding this comment.
Excellent demonstration of how to use mock objects (which I have not yet tried).
Some test method names I found difficult to understand. But then disocovered trying to come up with better alternatives was equally difficult. I finally worked out some suggested renames.
A variable set up in one test should be used in that test.
The level increment test doesn't appear to be testing the level increment.
tests/Unit/OutputFormatterTest.php
Outdated
| $renderedRenderable1 = 'tea'; | ||
| $renderable1->method('render')->with($this->outputFormat)->willReturn($renderedRenderable1); | ||
| $renderable2 = $this->createMock(Renderable::class); | ||
| $renderedRenderable2 = 'tea'; |
There was a problem hiding this comment.
Can I have coffee please? ;)
| /** | ||
| * @test | ||
| */ | ||
| public function implodeWithIncreaseLevelTrueIncreasesIndentationLevelForRendering(): void | ||
| { | ||
| $renderable = $this->createMock(Renderable::class); | ||
| $renderedRenderable = 'tea'; | ||
| $renderable->method('render')->with($this->outputFormat->nextLevel())->willReturn($renderedRenderable); | ||
| $values = [$renderable]; | ||
|
|
||
| $result = $this->subject->implode(', ', $values, true); | ||
|
|
||
| self::assertSame($renderedRenderable, $result); | ||
| } |
There was a problem hiding this comment.
I don't see the level increment being asserted here. Shouldn't we be getting tea after some space?
There was a problem hiding this comment.
No, that's up to the Rederable instance how it uses the indentation level. (For example, Comments do not care about the indentation level at all.)
What we're testing here in this unit tests is that render() will get the next level output format passed in: The with() call asserts that the expected call has the corresponding argument (compared using ==, not ===, or like assertEquals() instead of assertSame()), and will fail the test otherwise.
There was a problem hiding this comment.
I missed the change to the argument passed to with() compared with the previous test. I think I see and understand what's going on now. Reviewing this has been an educative process 👍
oliverklee
force-pushed
the
task/tests/output-formatter
branch
from
33db0fa to
e3a387c
Compare
Thanks! (Yes, I had a hard time coming up with good names for these cases, too …) |
Note: The new PHPStan warnings will go away once we add native type declarations for `OutputFormatter`. Part of #757
Co-authored-by: JakeQZ <[email protected]>
Co-authored-by: JakeQZ <[email protected]>
Co-authored-by: JakeQZ <[email protected]>
Co-authored-by: JakeQZ <[email protected]>
Co-authored-by: JakeQZ <[email protected]>
oliverklee
force-pushed
the
task/tests/output-formatter
branch
from
dfadbaf to
2d617c6
Compare
I think we need to clear this up. 😉 I prefer coffee (one cup in the morning, one after lunch), and black Darjeeling (between and after that). And when I need the extra bit of energy, I occasionally enjoy some Club-Mate (or something similar from a different brand). So, what's your preference? |
|
I also found this quite amusing:
|
Seems I misunderstood since you preferred tea in the tests.
Coffee with/after breakfast and lunch, and after evening dinner. Usually from a cafatiere. When I smoked, I would have half a cup with each cigarette too. :) |
Note: The new PHPStan warnings will go away once we add
native type declarations for
OutputFormatter.Part of #757