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

Add tests for ResultRendererFactory class #126

Merged
merged 2 commits into from Oct 4, 2016

Conversation

Projects
None yet
3 participants
@TheoKouzelis
Copy link
Contributor

TheoKouzelis commented Oct 3, 2016

No description provided.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 4, 2016

Current coverage is 88.80% (diff: 100%)

Merging #126 into master will increase coverage by 0.72%

@@             master       #126   diff @@
==========================================
  Files            69         69          
  Lines          1594       1546    -48   
  Methods         281        282     +1   
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           1404       1373    -31   
+ Misses          190        173    -17   
  Partials          0          0          

Powered by Codecov. Last update 6fa4db4...bc185c3

protected function isImplementationNameOfClass($implementationName, $className)
{
return is_string($implementationName) && is_subclass_of($implementationName, $className);

This comment has been minimized.

@AydinHassan

AydinHassan Oct 4, 2016

Member

hey @TheoKouzelis what is the advantage of is_subclass_of instead of class_implements ?

This comment has been minimized.

@TheoKouzelis

TheoKouzelis Oct 4, 2016

Contributor

I think class_implements returns an array of interfaces that the given class implements. http://php.net/manual/en/function.class-implements.php

So I think the previous code started to fail because the following statement would be true if the class implemented any interface and not specifically ResultInterface::class

!class_implements($resultClass, ResultInterface::class)

I think the code could have also been refactored to the following but I thought is_subclass_ofseemed more direct

!in_array(ResultInterface::class, class_implements($resultClass))

This comment has been minimized.

@AydinHassan

AydinHassan Oct 4, 2016

Member

oh wow - so it never worked 😂 - nice find!

@AydinHassan AydinHassan merged commit 65ba8a5 into php-school:master Oct 4, 2016

4 checks passed

Scrutinizer 1 new issues, 8 updated code elements
Details
codecov/patch 100% of diff hit (target 88.08%)
Details
codecov/project 88.80% (+0.72%) compared to 6fa4db4
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@AydinHassan

This comment has been minimized.

Copy link
Member

AydinHassan commented Oct 4, 2016

Thanks @TheoKouzelis !

@TheoKouzelis

This comment has been minimized.

Copy link
Contributor

TheoKouzelis commented Oct 4, 2016

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment