Skip to content
This repository has been archived by the owner on Sep 30, 2021. It is now read-only.

Added test for SonataCoreBundle #272

Merged

Conversation

codebach
Copy link
Contributor

@codebach codebach commented May 26, 2016

Subject

Writing test to increase code coverage of SonataCoreBundle

Todo

  • remove @group annotations after WIP
  • remove @Covers annotations
  • line break

}

$this->fail(sprintf('Compiler pass is not one of the expected types. Expects "Sonata\AdminBundle\DependencyInjection\Compiler\StatusRendererCompilerPass", "Sonata\AdminBundle\DependencyInjection\Compiler\AdapterCompilerPass" or "Sonata\AdminBundle\DependencyInjection\Compiler\FormFactoryCompilerPass", but got "%s".', get_class($pass)));
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

You SHOULD use Prophecy (SHOULD as in "the PR will not get refused it you don't, but it's better) ;)

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK, the discussion is not over: sonata-project/dev-kit#89 :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, my bad!

@greg0ire
Copy link
Contributor

Excellent initiative @ahmetakbn !

{
/**
* @group core-bundle
* @covers Sonata\CoreBundle\SonataCoreBundle::build
Copy link
Member

Choose a reason for hiding this comment

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

Please remove @covers. We want to follow Symfony usage and they don't use it.

@codebach
Copy link
Contributor Author

codebach commented Jun 2, 2016

The only lines not being tested are these in SonataCoreBundle.php

if (class_exists('Nelmio\ApiDocBundle\Form\Extension\DescriptionFormTypeExtension')) {
    $formTypes[] = 'nelmio_api_doc.form.extension.description_form_type_extension';
}

How should I mock php native functions? any suggestions? May be this: https://github.com/php-mock/php-mock

@greg0ire
Copy link
Contributor

greg0ire commented Jun 2, 2016

I think NelmioApiDocBundle should be added to require-dev. You need to rebase to fix StyleCI.

@codebach
Copy link
Contributor Author

codebach commented Jun 2, 2016

Ok, I will add NelmioApiDocBundle to require dev. The StyleCI is failing in other files which are not related to this PR. I will open another PR for that.

@soullivaneuh
Copy link
Member

No need for the PR, I'll do that with styleci.

Sullivan SENECHAL
Le 2 juin 2016 13:32, "Ahmet Akbana" notifications@github.com a écrit :

Ok, I will add NelmioApiDocBundle to require dev. The StyleCI is failing
in other files which are not related to this PR. I will open another PR for
that.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#272 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABnqNSlfm5MztYq9KHJtqbH7kUkbhKT3ks5qHr80gaJpZM4InsHv
.

@greg0ire
Copy link
Contributor

greg0ire commented Jun 2, 2016

Ok, I will add NelmioApiDocBundle to require dev. The StyleCI is failing in other files which are not related to this PR. I will open another PR for that.

Rebasing might be enough for that, I don't know…

@soullivaneuh
Copy link
Member

@ahmetakbn I checked StyleCI, nothing to be changed. Be sure your local 3.x branch is sync with upstream and do a rebase again.

@codebach codebach force-pushed the test-for-sonata-core-bundle branch from 1077b5b to fb5d4d3 Compare June 2, 2016 11:53
@soullivaneuh
Copy link
Member

Ouch. I think you did a rebase from master instead of 3.x...

@greg0ire
Copy link
Contributor

greg0ire commented Jun 2, 2016

git rebase -i origin/3.x can help you get rid of the unwanted commits

@codebach codebach force-pushed the test-for-sonata-core-bundle branch from f3d4aeb to eaac5f7 Compare June 2, 2016 13:23
@codebach
Copy link
Contributor Author

codebach commented Jun 2, 2016

Yes I didn't realize that, sorry. Now it is rebased to 3.x

@codebach codebach changed the title [WIP] Added test for SonataCoreBundle Added test for SonataCoreBundle Jun 2, 2016
@@ -37,7 +37,8 @@
"sensio/framework-extra-bundle": "^2.3 || ^3.0",
"sonata-project/exporter": "^1.3",
"symfony/phpunit-bridge": "^2.7",
"sllh/php-cs-fixer-styleci-bridge": "^2.0"
"sllh/php-cs-fixer-styleci-bridge": "^2.0",
"nelmio/api-doc-bundle": "^2.11"
Copy link
Member

Choose a reason for hiding this comment

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

why are we doing this in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comment

Copy link
Member

Choose a reason for hiding this comment

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

got it, sorry

@codebach codebach force-pushed the test-for-sonata-core-bundle branch from eaac5f7 to 24582ea Compare June 3, 2016 08:37
@codebach
Copy link
Contributor Author

codebach commented Jun 3, 2016

This PR is ready from my side.

/**
* @dataProvider getRegisteredFormMappingAndExtensions
*
* @param $mapping
Copy link
Member

Choose a reason for hiding this comment

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

Param type?

@greg0ire
Copy link
Contributor

greg0ire commented Jun 3, 2016

LGTM

@soullivaneuh
Copy link
Member

@ahmetakbn I removed the changelog, this is not needed just for test changes.

@soullivaneuh
Copy link
Member

In the name of the code coverage, I thank you! 👍

@soullivaneuh soullivaneuh merged commit 092c6b9 into sonata-project:3.x Jun 3, 2016
@greg0ire
Copy link
Contributor

greg0ire commented Jun 3, 2016

+5.4%

Nice!

@OskarStark
Copy link
Member

Thank you @ahmetakbn kepp the good work 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants