Skip to content

Fix unit tests#1631

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

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

Conversation

@vincentchalamon
Copy link
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
Copy link
Member

soyuka commented Jan 8, 2018

It's only failing on master?

@vincentchalamon
Copy link
Contributor Author

vincentchalamon commented Jan 8, 2018

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

@soyuka
Copy link
Member

soyuka commented Jan 8, 2018

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
Contributor Author

@soyuka It's rebased on 2.0

@soyuka
Copy link
Member

soyuka commented Jan 8, 2018

still failing with some deprecations :|

@vincentchalamon
Copy link
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
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
Copy link
Member

dunglas commented Jan 8, 2018

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