8000 asset test coverage by eventhorizonpl · Pull Request #16428 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

asset test coverage #16428

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

asset test coverage #16428

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shif 8000 t + click to select a range
fa43b7b
asset test coverage p1
Nov 2, 2015
642a5bf
address jakzal comments
Nov 2, 2015
7382dc0
address jakzal comments p2
Nov 2, 2015
c19c90b
address stof comment
Nov 3, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
asset test coverage p1 
asset test coverage p2

asset test coverage p3
  • Loading branch information
Michal Piotrowski committed Nov 2, 2015
commit fa43b7b9e4e56efada62f93e757bffb7f0c81f01
31 changes: 31 additions & 0 deletions src/Symfony/Component/Asset/Tests/Context/NullContextTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Asset\Tests\Context;

use Symfony\Component\Asset\Context\NullContext;

class NullContextTest extends \PHPUnit_Framework_TestCase
{
public function testGetBasePath()
{
$nullContext = new NullContext();

$this->assertEmpty($nullContext->getBasePath());
}

public function testIsSecure()
{
$nullContext = new NullContext();

$this->assertFalse($nullContext->isSecure());
}
}
72 changes: 72 additions & 0 deletions src/Symfony/Component/Asset/Tests/Context/RequestStackContextTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Asset\Tests\Context;

use Symfony\Component\Asset\Context\RequestStackContext;

class RequestStackContextTest extends \PHPUnit_Framework_TestCase
{
public function testConstructor()
{
$requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 3, 2015

Choose a reason for hiding this comment

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

Last occurrence of the mock builder :)

$requestStackContext = new RequestStackContext($requestStack);

$this->assertInstanceOf('Symfony\Component\Asset\Context\RequestStackContext', $requestStackContext);
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

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).

Copy link
Author
@eventhorizonpl eventhorizonpl Nov 2, 2015

Choose a reason for hiding this comment

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

something like that?
$this->assertInstanceOf('Symfony\Component\Asset\Context\ContextInterface', $requestStackContext);

Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

Yes 👍

I wouldn't call the method testConstructor, but sth like testItIsAContext

}

public function testGetBasePath()
{
$requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

This could simply be $requestStack = $this->getMock('Symfony\Component\HttpFoundation\RequestStack');.

$requestStackContext = new RequestStackContext($requestStack);

$this->assertEmpty($requestStackContext->getBasePath());
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

This should be the end of the test case. As a rule of thumb it's good to follow an arrange-act-assert flow (for readability and maintainability). I know there's lots of tests violating this rule in Symfony, but I'd rather don't add more of them.

This also shows a problem with naming test cases after a method being tested, as it basically forces you to put everything in a single method. I prefer to summarise the requirement/rule (for example: testItIsInitialisedWithAnEmptyBasePath, testItRetrievesTheBasePathFromTheMasterRequest).

Copy link
Author
@eventhorizonpl eventhorizonpl Nov 2, 2015

Choose a reason for hiding this comment

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

how about testGetBasePathEmpty + testGetBasePathSet and testIsSecureFalse + testIsSecureTrue? a bit shorter method names.


$testBasePath = 'test-path';

$request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

$request = $this->getMock('Symfony\Component\HttpFoundation\Request');

$request->method('getBasePath')
->willReturn($testBasePath);
$requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

$requestStack = $this->getMock('Symfony\Component\HttpFoundation\RequestStack');

$requestStack->method('getMasterRequest')
->willReturn($request);

$requestStackContext = new RequestStackContext($requestStack);

$this->assertEquals($testBasePath, $requestStackContext->getBasePath());
}

public function testIsSecure()
{
$requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

$requestStack = $this->getMock('Symfony\Component\HttpFoundation\RequestStack');

$requestStackContext = new RequestStackContext($requestStack);

$this->assertFalse($requestStackContext->isSecure());
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

Again, this should be the end of the test case. Following code is a separate one.


$request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

$request = $this->getMock('Symfony\Component\HttpFoundation\Request');

$request->method('isSecure')
->willReturn(true);
$requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')
->getMock();
Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

$requestStack = $this->getMock('Symfony\Component\HttpFoundation\RequestStack');

Copy link
Contributor
@jakzal jakzal Nov 2, 2015

Choose a reason for hiding this comment

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

I might have missed few occurrences, but you get the idea ;)

$requestStack->method('getMasterRequest')
->willReturn($request);

$requestStackContext = new RequestStackContext($requestStack);

$this->assertTrue($requestStackContext->isSecure());
}
}
33 changes: 33 additions & 0 deletions src/Symfony/Component/Asset/Tests/VersionStrategy/EmptyVersionStrategyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Asset\Tests\VersionStrategy;

use Symfony\Component\Asset\VersionStrategy\EmptyVersionStrategy;

class EmptyVersionStrategyTest extends \PHPUnit_Framework_TestCase
{
public function testGetVersion()
{
$emptyVersionStrategy = new EmptyVersionStrategy();
$path = 'test-path';

$this->assertEmpty($emptyVersionStrategy->getVersion($path));
}

public function testApplyVersion()
{
$emptyVersionStrategy = new EmptyVersionStrategy();
$path = 'test-path';

$this->assertEquals($path, $emptyVersionStrategy->applyVersion($path));
}
}
0