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

Bugfix issue #5163 #5236

Merged
merged 4 commits into from Dec 18, 2018
Merged

Bugfix issue #5163 #5236

merged 4 commits into from Dec 18, 2018

Conversation

dh9325
Copy link
Contributor

@dh9325 dh9325 commented Oct 17, 2018

@DavertMik Are you able to have a look at this and review? I understand @SamMousa suggested removing __mocked property but not sure if that's would be an "easy fix", so just checking if class is implementing PHPUnit\Framework\MockObject\MockObject interface.

Closes #5163

@@ -173,7 +174,7 @@ protected function getClassName($argument)
{
if ($argument instanceof \Closure) {
return 'Closure';
} elseif ((isset($argument->__mocked))) {
} elseif (in_array(MockObject::class, class_implements(get_class($argument)))) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use instanceof?

src/Codeception/Step.php Outdated Show resolved Hide resolved
Fix suggestion commit
@SamMousa
Copy link
Collaborator

LGTM, will merge when tests finish.

@samdark
Copy link
Collaborator

samdark commented Oct 17, 2018

New github feature is cool but it seems they have a bug :)

@@ -173,7 +174,7 @@ protected function getClassName($argument)
{
if ($argument instanceof \Closure) {
return 'Closure';
} elseif ((isset($argument->__mocked))) {
} elseif ($argument instanceof MockObject) {
return $this->formatClassName($argument->__mocked);
Copy link
Member

Choose a reason for hiding this comment

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

Ok, what if there is no __mocked property but an object is still MockObject?

Copy link
Contributor Author

@dh9325 dh9325 Nov 15, 2018

Choose a reason for hiding this comment

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

@DavertMik Have applied changes as requested but AppVeyor build failed (it looks like AppVeyor error)

elseif ($argument instanceof MockObject && isset($argument->__mocked) are you happy with this?

@DavertMik
Copy link
Member

Please don't mark the PR with [Yii] if it's not Yii related.

@DavertMik DavertMik changed the title [Yii2] Bugfix issue 5163 Bugfix issue 5163 Oct 18, 2018
@DavertMik DavertMik changed the title Bugfix issue 5163 Bugfix issue #5163 Oct 18, 2018
@DavertMik
Copy link
Member

I think the better way is to remove __mocked...
Sorry I have no time to investigate if it's spossible to do without concequinces

@dh9325
Copy link
Contributor Author

dh9325 commented Nov 16, 2018

@DavertMik Below is the AppVeyor error. Is there a way to re-trigger the build?

 - kb2999226 (exited 1058) - Error while running 'C:\ProgramData\chocolatey\lib\KB2999226\tools\chocolateyinstall.ps1'.
 See log for details.
 - kb2919442 (exited 1058) - Error while running 'C:\ProgramData\chocolatey\lib\KB2919442\tools\ChocolateyInstall.ps1'.
 See log for details.
 - kb3035131 (exited 1058) - Error while running 'C:\ProgramData\chocolatey\lib\KB3035131\Tools\ChocolateyInstall.ps1'.
 See log for details.
Command exited with code 1058

@dh9325
Copy link
Contributor Author

dh9325 commented Nov 20, 2018

@SamMousa @samdark Any chance to have a look at failing AppVeyor build? It doesn't seem to be related to the code changes. Thanks

@SamMousa
Copy link
Collaborator

Sorry, not familiar with AppVeyor

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.

[Yii2] Error: Call to a member function getDb() on null when running --html option
6 participants