asset test coverage #16428
asset test coverage #16428
Conversation
asset test coverage p2 asset test coverage p3
|
👍 |
| ->getMock(); | ||
| $requestStackContext = new RequestStackContext($requestStack); | ||
|
|
||
| $this->assertInstanceOf('Symfony\Component\Asset\Context\RequestStackContext', $requestStackContext); |
There was a problem hiding this comment.
This test is not really useful as all it does is it verifies if PHP works properly. All the other tests already cover this case.
I'd see some value in such a test case if it was verifying if a certain interface is implemented (it would communicate certain design decision).
There was a problem hiding this comment.
something like that?
$this->assertInstanceOf('Symfony\Component\Asset\Context\ContextInterface', $requestStackContext);
There was a problem hiding this comment.
Yes 👍
I wouldn't call the method testConstructor, but sth like testItIsAContext
|
Thanks for putting effort into adding more tests 👍 |
|
👍 |
| public function testItIsAContext() | ||
| { | ||
| $requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack') | ||
| ->getMock(); |
There was a problem hiding this comment.
Last occurrence of the mock builder :)
|
status: reviewed 👍 |
|
|
||
| class RequestStackContextTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
| public function testItIsAContext() |
There was a problem hiding this comment.
IMO, this test brings no value. We don't have such tests in Symfony (and you don't do it for NullContext btw)
|
For the merger: this should be probably merged into 2.7. status: reviewed |
|
Thank you @eventhorizonpl. |
This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #16428). Discussion ---------- asset test coverage Hi, This PR adds 100% tests code coverage for Asset component. Best regards, Michal | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT Commits ------- 5a6c1f2 asset test coverage
Hi,
This PR adds 100% tests code coverage for Asset component.
Best regards,
Michal