Skip to content

Conversation

samuelbigard
Copy link
Contributor

Q A
Branch? main
Type Feature
Tickets
License MIT

@samuelbigard samuelbigard force-pushed the feat/debug-resource-command branch from edeefa5 to 8e50f5a Compare August 31, 2021 15:21
@soyuka soyuka force-pushed the feat/debug-resource-command branch from 8e50f5a to 0c7cdb9 Compare September 1, 2021 06:24
@samuelbigard samuelbigard force-pushed the feat/debug-resource-command branch 2 times, most recently from 8f88ed1 to af157d1 Compare September 1, 2021 10:12
@samuelbigard samuelbigard force-pushed the feat/debug-resource-command branch 2 times, most recently from 44e7e1d to 8228db1 Compare September 2, 2021 15:17

public function testDebugResource()
{
$varDumper = $this->prophesize(CliDumper::class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it's not necessary to mock this class (don't mock what you don't own). You may just instantiate a CliDumper dumping to /dev/null.

This should do the trick.

Suggested change
$varDumper = $this->prophesize(CliDumper::class);
$varDumper = new CliDumper('Windows' === PHP_OS_FAMILY ? 'NUL' : '/dev/null');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we want to test that dump is called isn't that alright?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should at least mock the interface instead of a concretion in this case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option could be to set php://memory as output and to assert that something has been written in it.

@samuelbigard samuelbigard force-pushed the feat/debug-resource-command branch from 8228db1 to b7a8662 Compare September 3, 2021 07:39
@samuelbigard samuelbigard force-pushed the feat/debug-resource-command branch from b7a8662 to b63200c Compare September 6, 2021 19:45
@soyuka soyuka merged commit 334914a into api-platform:main Sep 7, 2021
@soyuka
Copy link
Member

soyuka commented Sep 7, 2021

Thanks @samuelbigard !

@samuelbigard samuelbigard deleted the feat/debug-resource-command branch September 7, 2021 08:31
vincentchalamon pushed a commit to vincentchalamon/core that referenced this pull request Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants