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

[WIP][ResourceBundle] Create integration tests #4064

Merged
merged 1 commit into from Feb 18, 2016

Conversation

Projects
None yet
5 participants
@tuka217
Copy link
Contributor

commented Feb 5, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
License MIT
Doc PR -

@tuka217 tuka217 force-pushed the tuka217:test-resource-bundle branch from 2eb8da9 to 4792a2d Feb 5, 2016

@@ -35,10 +35,13 @@
"symfony/twig-bundle": "^2.7",
"white-october/pagerfanta-bundle": "^1.0",
"willdurand/hateoas-bundle": "^0.4|^1.0"

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Redundant blank line

"psr-4": { "Sylius\\Bundle\\ResourceBundle\\": "" }
"psr-4": {
"Sylius\\Bundle\\ResourceBundle\\": "",
"AppBundle\\": "tests/src/AppBundle/"

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

This should go to autoload-dev

<directory>./tests/</directory>
</testsuite>
</testsuites>
</phpunit>

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Missing blank line at the end of file

$loader = require __DIR__.'/../../vendor/autoload.php';
require __DIR__.'/AppKernel.php';

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Missing blank line

*/
public function registerBundles()
{
return array(

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Remember about short syntax

resource: %kernel.root_dir%/routing.yml
session: ~
form: ~
templating: { engines: ['twig'] }

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Too many spaces

sylius_resource_controller:
resource: |
alias: sylius.resource_controller
type: sylius.resource

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Missing blank line

* @var string
*/
private $title;

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Redundant blank line

*/
class AppBundle extends Bundle
{

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Redundant blank line

alias: app.book
type: sylius.resource

sylius_resource_controller:

This comment has been minimized.

Copy link
@lchrusciel

lchrusciel Feb 5, 2016

Member

Why do you register crud for sylius_resource_controller?

@@ -3,3 +3,6 @@ bin/

composer.phar
composer.lock

tests/app/cache

This comment has been minimized.

Copy link
@pamil

pamil Feb 5, 2016

Member

Why don't we use a temporary directory just like in ThemeBundle/Tests/Functional/WebTestCase::getTmpDirPath?

This comment has been minimized.

Copy link
@tuka217

tuka217 Feb 8, 2016

Author Contributor

We don't use this method in ThemeBundle. Are you sure that you saw this in .gitignore file?

new WhiteOctober\PagerfantaBundle\WhiteOctoberPagerfantaBundle(),
new Bazinga\Bundle\HateoasBundle\BazingaHateoasBundle(),
new Doctrine\Bundle\DoctrineBundle\DoctrineBundle(),
new \Symfony\Bundle\TwigBundle\TwigBundle(),

This comment has been minimized.

Copy link
@pamil

pamil Feb 5, 2016

Member

No \ needed at the beginning.

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Bundle\Bundle;

This comment has been minimized.

Copy link
@pamil

pamil Feb 5, 2016

Member

Doubled blank line

}
/**
* @param $title

This comment has been minimized.

Copy link
@pamil

pamil Feb 5, 2016

Member

Missing typehint

*/
class BookType extends AbstractResourceType
{
public function buildForm(FormBuilderInterface $builder, array $options)

This comment has been minimized.

Copy link
@pamil

pamil Feb 5, 2016

Member

Missing inhertidocs

@pamil

This comment has been minimized.

Copy link
Member

commented Feb 5, 2016

@tuka217 tuka217 force-pushed the tuka217:test-resource-bundle branch 7 times, most recently from edc6f0e to eda0644 Feb 8, 2016

new Symfony\Bundle\TwigBundle\TwigBundle(),
new AppBundle\AppBundle(),
new Sensio\Bundle\GeneratorBundle\SensioGeneratorBundle(),
new Symfony\Bundle\MonologBundle\MonologBundle(),

This comment has been minimized.

Copy link
@pjedrzejewski

pjedrzejewski Feb 12, 2016

Member

We do not need Monolog?

new AppBundle\AppBundle(),
new Sensio\Bundle\GeneratorBundle\SensioGeneratorBundle(),
new Symfony\Bundle\MonologBundle\MonologBundle(),
new Sensio\Bundle\DistributionBundle\SensioDistributionBundle(),

This comment has been minimized.

Copy link
@pjedrzejewski

pjedrzejewski Feb 12, 2016

Member

Do we need this bundle?

strict_requirements: ~
form: ~
csrf_protection: ~
validation: { enable_annotations: true }

This comment has been minimized.

Copy link
@pjedrzejewski

pjedrzejewski Feb 12, 2016

Member

Not needed?

secret: ThisTokenIsNotSoSecretChangeIt
debug_toolbar: true
debug_redirects: false
use_assetic_controller: true

This comment has been minimized.

Copy link
@pjedrzejewski

pjedrzejewski Feb 12, 2016

Member

Not needed?

@tuka217 tuka217 force-pushed the tuka217:test-resource-bundle branch from eda0644 to f455622 Feb 12, 2016

@tuka217 tuka217 force-pushed the tuka217:test-resource-bundle branch 11 times, most recently from 1a42f0f to c5506ea Feb 12, 2016

[ResourceBundle] Add configuration test
[ResourceBundle] Add api test

@tuka217 tuka217 force-pushed the tuka217:test-resource-bundle branch from c5506ea to 1c290ee Feb 18, 2016

new Symfony\Bundle\TwigBundle\TwigBundle(),
new Sensio\Bundle\GeneratorBundle\SensioGeneratorBundle(),
new AppBundle\AppBundle(),

This comment has been minimized.

Copy link
@Zales0123

Zales0123 Feb 18, 2016

Member

Redundant blank line.

protected function getContainerBaseClass()
{
if ('test' === $this->environment) {
return '\PSS\SymfonyMockerContainer\DependencyInjection\MockerContainer';

This comment has been minimized.

Copy link
@Zales0123

Zales0123 Feb 18, 2016

Member

Can't we use MockerContainer::class?

pjedrzejewski added a commit that referenced this pull request Feb 18, 2016

Merge pull request #4064 from tuka217/test-resource-bundle
[WIP][ResourceBundle] Create integration tests

@pjedrzejewski pjedrzejewski merged commit 496caa1 into Sylius:master Feb 18, 2016

2 checks passed

Scrutinizer 1 new issues, 10 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pjedrzejewski

This comment has been minimized.

Copy link
Member

commented Feb 18, 2016

Nice work Ania, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.