-
Notifications
You must be signed in to change notification settings - Fork 461
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
Easy testing for traits #685
Easy testing for traits #685
Conversation
@@ -467,6 +498,14 @@ protected function getAllMethods() | |||
$methods = array_merge($methods, $class->getMethods()); | |||
} | |||
|
|||
foreach ($this->getTargetTraits() AS $trait) { | |||
foreach ($trait->getMethods() as $method) { | |||
if ($method->isAbstract()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a while since I last looked at mockery's internals, but shouldn't you call isTrait
here? If not, why do we need the isTrait
method? I don't see it called anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't need calling there and therefore you are right, we don't need it for this implementation. I put it in there thinking I would, will back it out.
I don't think this has any bearing on this functionality, but maybe someone else might think of something, there is an RFC currently under discussion to bring interfaces to traits - https://wiki.php.net/rfc/traits-with-interfaces. |
b8f1639
to
995d92b
Compare
Thought I'd have a crack at this to see what it looked like, a few people have whined about it in #200. It's not really what I'd consider a responsibility of a test double framework, but we have the tools to do it...