Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix specs for MethodProphecy #434

Merged
merged 1 commit into from
Aug 23, 2019
Merged

Fix specs for MethodProphecy #434

merged 1 commit into from
Aug 23, 2019

Conversation

tkotosz
Copy link
Contributor

@tkotosz tkotosz commented Jun 27, 2019

Currently when running phpscpec for the MethodProphecy class then I get the following:

$ vendor/bin/phpspec r spec/Prophecy/Prophecy/MethodProphecySpec.php 
                                                                                 0
0 specs
0 examples 
0ms

It seems this happens because a new class is defined before the MethodProphecySpec class which seemingly not allowed.

I have moved that other class to the bottom of the file then the tests started to run but a few of them was failing so I fixed those in this PR.

after the fixes:
image

@tkotosz
Copy link
Contributor Author

tkotosz commented Jun 27, 2019

php 5 build fixed now, please let me now if there is a better way to fix it :)


class ClassWithVoidTypeHintedMethods
{
public function getVoid()
Copy link
Member

Choose a reason for hiding this comment

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

Should rather be dropped entirely, as this class does not actually have void typehinted methods, and skipping this case in the specs on PHP 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will see how to create a double manually instead of asking in it as a method argument (like now in its_constructor_records_default_callback_promise_for_return_type_hinted_methods) otherwise the php5 build would fail since the ClassWithVoidTypeHintedMethods class would be missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, managed to work around it

@tkotosz
Copy link
Contributor Author

tkotosz commented Jul 17, 2019

@stof fixed the PR based on your feedback. Could you please have a look again?

@tkotosz
Copy link
Contributor Author

tkotosz commented Jul 18, 2019

squashed commits to prep the PR for merge

@stof stof merged commit 40c22e8 into phpspec:master Aug 23, 2019
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.

2 participants