Skip to content

Fix unit tests#1631

Closed
vincentchalamon wants to merge 1 commit into
api-platform:2.0from
vincentchalamon:master
Closed

Fix unit tests#1631
vincentchalamon wants to merge 1 commit into
api-platform:2.0from
vincentchalamon:master

Conversation

@vincentchalamon

Copy link
Copy Markdown
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1580 (comment)
License MIT
Doc PR

@soyuka

soyuka commented Jan 8, 2018

Copy link
Copy Markdown
Member

It's only failing on master?

@vincentchalamon

vincentchalamon commented Jan 8, 2018

Copy link
Copy Markdown
Contributor Author

@soyuka I don't think so.
It's related to this: symfony/symfony@0917c4c#diff-9d63a61ac1b3720a090df6b1015822f2
Maybe should I target to 2.0?

@soyuka

soyuka commented Jan 8, 2018

Copy link
Copy Markdown
Member

Maybe should I target to 2.0?

Yes

@vincentchalamon vincentchalamon changed the base branch from master to 2.0 January 8, 2018 16:04
@vincentchalamon

Copy link
Copy Markdown
Contributor Author

@soyuka It's rebased on 2.0

@soyuka

soyuka commented Jan 8, 2018

Copy link
Copy Markdown
Member

still failing with some deprecations :|

@vincentchalamon

Copy link
Copy Markdown
Contributor Author

@soyuka Those deprecations are out of this PR scope. Was it working earlier? Should I target it on 2.1?

{
$request = new Request();

$requestProphecy = $this->prophesize(Request::class);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Objects (without related interfaces) from external libraries should not be mocked, especially when its a value object like this one.

@dunglas dunglas mentioned this pull request Jan 8, 2018
@dunglas

dunglas commented Jan 8, 2018

Copy link
Copy Markdown
Member

Thanks @vincentchalamon, fixed in #1634.

@dunglas dunglas closed this Jan 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants