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

Test cart #8364

Merged
merged 11 commits into from Oct 30, 2017

Conversation

Projects
None yet
5 participants
@tomlev
Member

tomlev commented Sep 25, 2017

Questions Answers
Branch? develop
Description? this PR adds some tests on cart add and calculation
Type? improvement
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? run unit tests

Important guidelines


This change is Reviewable

@eternoendless eternoendless changed the base branch from 1.7.2.x to develop Oct 12, 2017

@eternoendless eternoendless changed the title from TE: test cart to Test cart Oct 12, 2017

@eternoendless eternoendless added this to the 1.7.3.0 milestone Oct 12, 2017

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 13, 2017

Contributor

Review status: 0 of 5 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

        $this->contextMocker = new ContextMocker();
        $this->contextMocker->mockContext();
        $this->cart              = new \Cart();

Please Import all the classes you use (I won't spam the whole review with this request)


tests/Unit/Core/Cart/AbstractCartTest.php, line 107 at r1 (raw file):

        parent::tearDown();

        $this->resetCart();

Why reset cart in tearDown() if you do it in setUp() anyway ?


tests/Unit/Core/Cart/AbstractCartTest.php, line 120 at r1 (raw file):


        // delete cart
        $this->cart->delete();

If cart is deleted here, why reset it just before ?


tests/Unit/Core/Cart/AbstractCartTest.php, line 139 at r1 (raw file):

        $productDatas = $this->cart->getProducts(true);
        foreach ($productDatas as $productData) {
            $this->cart->updateQty(0, $productData['id_product']);

Cart::updateQty() instantiates a bunch of objects and triggers a hook (but maybe this is intended ?).
Cart::deleteProduct() though, only removes product from cart.


tests/Unit/Core/Cart/AbstractCartTest.php, line 165 at r1 (raw file):

    }

    protected function addProductsToCart($productDatas)

"data" is already a plural world. No "s" needed.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 42 at r1 (raw file):

     * @param $expectedProductCountAfterRules
     */
    public function testCartRuleValidity(

Please add a description here (Given - When - Then). I had a very hard time understanding what this test does (and I'm still unsure).


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 43 at r1 (raw file):

     */
    public function testCartRuleValidity(
        $productDatas,

$productData


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 44 at r1 (raw file):

    public function testCartRuleValidity(
        $productDatas,
        $cartRuleDatas,

$cartRuleData


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 37 at r1 (raw file):

     * @dataProvider cartWithoutCartRulesProvider
     */
    public function testCartWithoutCartRules($productDatas, $expectedTotal, $cartRuleDatas)

"data"


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 42 at r1 (raw file):

        $this->addCartRulesToCart($cartRuleDatas);
        $this->compareCartTotal($expectedTotal);
    }

No assertions ? The test will be flagged as risky.


Comments from Reviewable

Contributor

LittleBigDev commented Oct 13, 2017

Review status: 0 of 5 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

        $this->contextMocker = new ContextMocker();
        $this->contextMocker->mockContext();
        $this->cart              = new \Cart();

Please Import all the classes you use (I won't spam the whole review with this request)


tests/Unit/Core/Cart/AbstractCartTest.php, line 107 at r1 (raw file):

        parent::tearDown();

        $this->resetCart();

Why reset cart in tearDown() if you do it in setUp() anyway ?


tests/Unit/Core/Cart/AbstractCartTest.php, line 120 at r1 (raw file):


        // delete cart
        $this->cart->delete();

If cart is deleted here, why reset it just before ?


tests/Unit/Core/Cart/AbstractCartTest.php, line 139 at r1 (raw file):

        $productDatas = $this->cart->getProducts(true);
        foreach ($productDatas as $productData) {
            $this->cart->updateQty(0, $productData['id_product']);

Cart::updateQty() instantiates a bunch of objects and triggers a hook (but maybe this is intended ?).
Cart::deleteProduct() though, only removes product from cart.


tests/Unit/Core/Cart/AbstractCartTest.php, line 165 at r1 (raw file):

    }

    protected function addProductsToCart($productDatas)

"data" is already a plural world. No "s" needed.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 42 at r1 (raw file):

     * @param $expectedProductCountAfterRules
     */
    public function testCartRuleValidity(

Please add a description here (Given - When - Then). I had a very hard time understanding what this test does (and I'm still unsure).


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 43 at r1 (raw file):

     */
    public function testCartRuleValidity(
        $productDatas,

$productData


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 44 at r1 (raw file):

    public function testCartRuleValidity(
        $productDatas,
        $cartRuleDatas,

$cartRuleData


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 37 at r1 (raw file):

     * @dataProvider cartWithoutCartRulesProvider
     */
    public function testCartWithoutCartRules($productDatas, $expectedTotal, $cartRuleDatas)

"data"


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 42 at r1 (raw file):

        $this->addCartRulesToCart($cartRuleDatas);
        $this->compareCartTotal($expectedTotal);
    }

No assertions ? The test will be flagged as risky.


Comments from Reviewable

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 13, 2017

Contributor

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.


Comments from Reviewable

Contributor

LittleBigDev commented Oct 13, 2017

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.


Comments from Reviewable

@LittleBigDev

See Reviewable comments

@tomlev

This comment has been minimized.

Show comment
Hide comment
@tomlev

tomlev Oct 19, 2017

Member

Review status: 0 of 24 files reviewed at latest revision, 10 unresolved discussions.


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Please Import all the classes you use (I won't spam the whole review with this request)

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 107 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Why reset cart in tearDown() if you do it in setUp() anyway ?

because some other tests dont sanitize the data they use


tests/Unit/Core/Cart/AbstractCartTest.php, line 120 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

If cart is deleted here, why reset it just before ?

reset is emptying cart, delete drop it


tests/Unit/Core/Cart/AbstractCartTest.php, line 139 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Cart::updateQty() instantiates a bunch of objects and triggers a hook (but maybe this is intended ?).
Cart::deleteProduct() though, only removes product from cart.

it is intended as it is the method used by cart update


tests/Unit/Core/Cart/AbstractCartTest.php, line 165 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

"data" is already a plural world. No "s" needed.

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 42 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Please add a description here (Given - When - Then). I had a very hard time understanding what this test does (and I'm still unsure).

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 43 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

$productData

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 44 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

$cartRuleData

Done.


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 37 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

"data"

Done.


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 42 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

No assertions ? The test will be flagged as risky.

assertions are in the methods (code factorization)


Comments from Reviewable

Member

tomlev commented Oct 19, 2017

Review status: 0 of 24 files reviewed at latest revision, 10 unresolved discussions.


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Please Import all the classes you use (I won't spam the whole review with this request)

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 107 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Why reset cart in tearDown() if you do it in setUp() anyway ?

because some other tests dont sanitize the data they use


tests/Unit/Core/Cart/AbstractCartTest.php, line 120 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

If cart is deleted here, why reset it just before ?

reset is emptying cart, delete drop it


tests/Unit/Core/Cart/AbstractCartTest.php, line 139 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Cart::updateQty() instantiates a bunch of objects and triggers a hook (but maybe this is intended ?).
Cart::deleteProduct() though, only removes product from cart.

it is intended as it is the method used by cart update


tests/Unit/Core/Cart/AbstractCartTest.php, line 165 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

"data" is already a plural world. No "s" needed.

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 42 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Please add a description here (Given - When - Then). I had a very hard time understanding what this test does (and I'm still unsure).

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 43 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

$productData

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 44 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

$cartRuleData

Done.


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 37 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

"data"

Done.


tests/Unit/Core/Cart/Calculation/Products/CartProductTest.php, line 42 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

No assertions ? The test will be flagged as risky.

assertions are in the methods (code factorization)


Comments from Reviewable

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 20, 2017

Contributor

Reviewed 24 of 24 files at r2.
Review status: all files reviewed at latest revision, 27 unresolved discussions.


classes/Product.php, line 3638 at r2 (raw file):

        if (!$ps_stock_management) {
            return true;
        } else {

the else statement can be removed because of the return true just above


tests/Integration/ProductURLsTest.php, line 40 at r2 (raw file):

    private $language;

    protected function setup()

did you mean setUp() ?


tests/Integration/ProductURLsTest.php, line 42 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/SmartySettingsTest.php, line 36 at r2 (raw file):

    private $smarty;

    public function setup()

did you mean setUp() ?


tests/Integration/SmartySettingsTest.php, line 38 at r2 (raw file):

    public function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Adapter/AdapterDatabaseTest.php, line 34 at r2 (raw file):

class AdapterDatabaseTest extends IntegrationTestCase
{
    public function setup()

did you mean setUp() ?


tests/Integration/Adapter/AdapterDatabaseTest.php, line 36 at r2 (raw file):

    public function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/classes/CartGetOrderTotalTest.php, line 371 at r2 (raw file):

    public function setUp()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/classes/ConfigurationCoreTest.php, line 39 at r2 (raw file):

    protected function setUp()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/classes/ShopCoreTest.php, line 37 at r2 (raw file):

    protected $context;

    protected function setup()

did you mean setUp() ?


tests/Integration/classes/ShopCoreTest.php, line 39 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 47 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 49 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 41 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 43 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Core/Module/HookRepositoryTest.php, line 46 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/Integration/Core/Module/HookRepositoryTest.php, line 48 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/TestCase/IntegrationTestCase.php, line 40 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/TestCase/IntegrationTestCase.php, line 42 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

Previously, tomlev (Thomas LEVIANDIER) wrote…

Done.

I still see \Cart and \Context here


tests/Unit/Core/Cart/AbstractCartTest.php, line 90 at r2 (raw file):

        parent::setUp();
        $this->cart              = new \Cart();
        $this->cart->id_lang     = (int) \Context::getContext()->language->id;

\Context too


tests/Unit/Core/Cart/AbstractCartTest.php, line 145 at r2 (raw file):

    {
        foreach ($this->productFixtures as $k => $productFixture) {
            $product           = new \Product;

Import Product


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 74 at r2 (raw file):

        foreach ($cartRuleData as $cartRuleId) {
            $cartRule                = $this->getCartRuleFromFixtureId($cartRuleId);
            $result                  = $result && $cartRule->checkValidity(\Context::getContext(), false, false);

Import \Context


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 56 at r2 (raw file):

    protected $contextMocker;

    public function setup()

did you mean setUp() ?


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 62 at r2 (raw file):

        $this->contextMocker->mockContext();

        parent::setup();

did you mean setUp() ?


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 78 at r2 (raw file):

    public function testDefaultMultishopContext()
    {
        \Shop::resetContext();

Import Shop


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 129 at r2 (raw file):

        $this->commandListener->onConsoleCommand($event);
    }
}

Missing final single blank line


Comments from Reviewable

Contributor

LittleBigDev commented Oct 20, 2017

Reviewed 24 of 24 files at r2.
Review status: all files reviewed at latest revision, 27 unresolved discussions.


classes/Product.php, line 3638 at r2 (raw file):

        if (!$ps_stock_management) {
            return true;
        } else {

the else statement can be removed because of the return true just above


tests/Integration/ProductURLsTest.php, line 40 at r2 (raw file):

    private $language;

    protected function setup()

did you mean setUp() ?


tests/Integration/ProductURLsTest.php, line 42 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/SmartySettingsTest.php, line 36 at r2 (raw file):

    private $smarty;

    public function setup()

did you mean setUp() ?


tests/Integration/SmartySettingsTest.php, line 38 at r2 (raw file):

    public function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Adapter/AdapterDatabaseTest.php, line 34 at r2 (raw file):

class AdapterDatabaseTest extends IntegrationTestCase
{
    public function setup()

did you mean setUp() ?


tests/Integration/Adapter/AdapterDatabaseTest.php, line 36 at r2 (raw file):

    public function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/classes/CartGetOrderTotalTest.php, line 371 at r2 (raw file):

    public function setUp()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/classes/ConfigurationCoreTest.php, line 39 at r2 (raw file):

    protected function setUp()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/classes/ShopCoreTest.php, line 37 at r2 (raw file):

    protected $context;

    protected function setup()

did you mean setUp() ?


tests/Integration/classes/ShopCoreTest.php, line 39 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 47 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 49 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 41 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 43 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Integration/Core/Module/HookRepositoryTest.php, line 46 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/Integration/Core/Module/HookRepositoryTest.php, line 48 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/TestCase/IntegrationTestCase.php, line 40 at r2 (raw file):

    protected $contextMocker;

    protected function setup()

did you mean setUp() ?


tests/TestCase/IntegrationTestCase.php, line 42 at r2 (raw file):

    protected function setup()
    {
        parent::setup();

did you mean setUp() ?


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

Previously, tomlev (Thomas LEVIANDIER) wrote…

Done.

I still see \Cart and \Context here


tests/Unit/Core/Cart/AbstractCartTest.php, line 90 at r2 (raw file):

        parent::setUp();
        $this->cart              = new \Cart();
        $this->cart->id_lang     = (int) \Context::getContext()->language->id;

\Context too


tests/Unit/Core/Cart/AbstractCartTest.php, line 145 at r2 (raw file):

    {
        foreach ($this->productFixtures as $k => $productFixture) {
            $product           = new \Product;

Import Product


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 74 at r2 (raw file):

        foreach ($cartRuleData as $cartRuleId) {
            $cartRule                = $this->getCartRuleFromFixtureId($cartRuleId);
            $result                  = $result && $cartRule->checkValidity(\Context::getContext(), false, false);

Import \Context


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 56 at r2 (raw file):

    protected $contextMocker;

    public function setup()

did you mean setUp() ?


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 62 at r2 (raw file):

        $this->contextMocker->mockContext();

        parent::setup();

did you mean setUp() ?


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 78 at r2 (raw file):

    public function testDefaultMultishopContext()
    {
        \Shop::resetContext();

Import Shop


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 129 at r2 (raw file):

        $this->commandListener->onConsoleCommand($event);
    }
}

Missing final single blank line


Comments from Reviewable

@LittleBigDev

LittleBigDev suggested changes Oct 20, 2017 edited

Thanks for the hard work.
Still some changes are missing tho. (see reviewable comments)

@tomlev

This comment has been minimized.

Show comment
Hide comment
@tomlev

tomlev Oct 20, 2017

Member

Review status: 11 of 41 files reviewed at latest revision, 27 unresolved discussions.


classes/Product.php, line 3638 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

the else statement can be removed because of the return true just above

Done.


tests/Integration/ProductURLsTest.php, line 40 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/ProductURLsTest.php, line 42 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/SmartySettingsTest.php, line 36 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/SmartySettingsTest.php, line 38 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Adapter/AdapterDatabaseTest.php, line 34 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Adapter/AdapterDatabaseTest.php, line 36 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/CartGetOrderTotalTest.php, line 371 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/ConfigurationCoreTest.php, line 39 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/ShopCoreTest.php, line 37 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/ShopCoreTest.php, line 39 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 47 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 49 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 41 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 43 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Module/HookRepositoryTest.php, line 46 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Module/HookRepositoryTest.php, line 48 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/TestCase/IntegrationTestCase.php, line 40 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/TestCase/IntegrationTestCase.php, line 42 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

I still see \Cart and \Context here

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 90 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

\Context too

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 145 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Import Product

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 74 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Import \Context

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 56 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 62 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 78 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Import Shop

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 129 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Missing final single blank line

Done.


Comments from Reviewable

Member

tomlev commented Oct 20, 2017

Review status: 11 of 41 files reviewed at latest revision, 27 unresolved discussions.


classes/Product.php, line 3638 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

the else statement can be removed because of the return true just above

Done.


tests/Integration/ProductURLsTest.php, line 40 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/ProductURLsTest.php, line 42 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/SmartySettingsTest.php, line 36 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/SmartySettingsTest.php, line 38 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Adapter/AdapterDatabaseTest.php, line 34 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Adapter/AdapterDatabaseTest.php, line 36 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/CartGetOrderTotalTest.php, line 371 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/ConfigurationCoreTest.php, line 39 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/ShopCoreTest.php, line 37 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/classes/ShopCoreTest.php, line 39 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 47 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityManagerTest.php, line 49 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 41 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Foundation/Entity/EntityTest.php, line 43 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Module/HookRepositoryTest.php, line 46 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Integration/Core/Module/HookRepositoryTest.php, line 48 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/TestCase/IntegrationTestCase.php, line 40 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/TestCase/IntegrationTestCase.php, line 42 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 93 at r1 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

I still see \Cart and \Context here

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 90 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

\Context too

Done.


tests/Unit/Core/Cart/AbstractCartTest.php, line 145 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Import Product

Done.


tests/Unit/Core/Cart/Adding/CartRule/AddRuleTest.php, line 74 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Import \Context

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 56 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 62 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

did you mean setUp() ?

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 78 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Import Shop

Done.


tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php, line 129 at r2 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Missing final single blank line

Done.


Comments from Reviewable

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 20, 2017

Contributor

Reviewed 30 of 30 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


tests/Unit/classes/module/ModuleCoreTest.php, line 51 at r3 (raw file):

										<ul><li>Error 1</li><li>Error 2</li><li>Error 3</li></ul>
									</div>
								</div>';

Care about tabulations


Comments from Reviewable

Contributor

LittleBigDev commented Oct 20, 2017

Reviewed 30 of 30 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


tests/Unit/classes/module/ModuleCoreTest.php, line 51 at r3 (raw file):

										<ul><li>Error 1</li><li>Error 2</li><li>Error 3</li></ul>
									</div>
								</div>';

Care about tabulations


Comments from Reviewable

@LittleBigDev

Almost there !

@tomlev

This comment has been minimized.

Show comment
Hide comment
@tomlev

tomlev Oct 20, 2017

Member

Review status: all files reviewed at latest revision, 1 unresolved discussion.


tests/Unit/classes/module/ModuleCoreTest.php, line 51 at r3 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Care about tabulations

these lines were not modified in this PR ;)


Comments from Reviewable

Member

tomlev commented Oct 20, 2017

Review status: all files reviewed at latest revision, 1 unresolved discussion.


tests/Unit/classes/module/ModuleCoreTest.php, line 51 at r3 (raw file):

Previously, LittleBigDev (Bastien Bieri) wrote…

Care about tabulations

these lines were not modified in this PR ;)


Comments from Reviewable

@codacy-bot

This comment has been minimized.

Show comment
Hide comment
@codacy-bot

codacy-bot Oct 20, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- tests/Integration/Core/Module/HookRepositoryTest.php  1
- tests/TestCase/IntegrationTestCase.php  2
- tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php  1
- tests/Integration/classes/CartGetOrderTotalTest.php  3
- classes/cache/Cache.php  1
- classes/Carrier.php  1
- tests/Integration/Core/Foundation/Entity/EntityTest.php  2
- tests/Integration/Core/Foundation/Entity/EntityManagerTest.php  1
- classes/shop/Shop.php  1
         

Complexity decreasing per file
==============================
+ classes/Product.php  -2
+ classes/Cart.php  -1
         

See the complete overview on Codacy

codacy-bot commented Oct 20, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- tests/Integration/Core/Module/HookRepositoryTest.php  1
- tests/TestCase/IntegrationTestCase.php  2
- tests/Unit/PrestaShopBundle/MultishopCommandListenerTest.php  1
- tests/Integration/classes/CartGetOrderTotalTest.php  3
- classes/cache/Cache.php  1
- classes/Carrier.php  1
- tests/Integration/Core/Foundation/Entity/EntityTest.php  2
- tests/Integration/Core/Foundation/Entity/EntityManagerTest.php  1
- classes/shop/Shop.php  1
         

Complexity decreasing per file
==============================
+ classes/Product.php  -2
+ classes/Cart.php  -1
         

See the complete overview on Codacy

@PrestaShop PrestaShop deleted a comment from codacy-bot Oct 20, 2017

@LittleBigDev

This comment has been minimized.

Show comment
Hide comment
@LittleBigDev

LittleBigDev Oct 24, 2017

Contributor

:lgtm:


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

Contributor

LittleBigDev commented Oct 24, 2017

:lgtm:


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@marionf marionf removed the waiting for QA label Oct 26, 2017

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless

eternoendless Oct 30, 2017

Member

Thank you @tomlev ! 🍰

Member

eternoendless commented Oct 30, 2017

Thank you @tomlev ! 🍰

@eternoendless eternoendless merged commit 5bea454 into PrestaShop:develop Oct 30, 2017

3 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/reviewable 41 files reviewed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment