From 86000cea24a9dd757e1b4d6cf54957e32e577f3c Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 23 Jul 2020 11:23:22 +0200 Subject: [PATCH 1/2] repair \Magento\Captcha\Test\Unit\Model\Customer\Plugin\AjaxLoginTest Data providers are executed before setUp. In previous phpunit versions $this->formIds[0] would just convert to null, explaining why the logAttempt method was never called. Fixing this issue leads to different expectations of the test result in testAroundExecuteCaptchaIsNotRequired. I have adjusted the necessary. No need to test if this method returns nothing if the return type is already void Fix quote unit tests by explicitely declaring methods that are vital to quote operations. I think it is better to declare these methods explicitely than to depend on DataObject methods Update extensionattribute mock repair partial mock with non-existing method mocking Revert "Fix quote unit tests by explicitely declaring methods that are vital" This reverts commit e29506ce3b353d55cbe64d064e863c4969c3d6cd. fix mock object with non existing method mocks replace partial mocks with non-existing method mocking --- .../Model/Customer/Plugin/AjaxLoginTest.php | 8 ++-- .../Command/XmlCatalogGenerateCommandTest.php | 2 +- .../Test/Unit/Model/QuoteManagerTest.php | 11 ++--- .../Quote/Address/Total/SubtotalTest.php | 45 ++++++++++++------- .../Model/Order/CreditmemoFactoryTest.php | 12 +++-- .../Test/Unit/Adjustment/CollectionTest.php | 19 ++++---- 6 files changed, 56 insertions(+), 41 deletions(-) diff --git a/app/code/Magento/Captcha/Test/Unit/Model/Customer/Plugin/AjaxLoginTest.php b/app/code/Magento/Captcha/Test/Unit/Model/Customer/Plugin/AjaxLoginTest.php index f243718952f4f..3c5aed076a653 100644 --- a/app/code/Magento/Captcha/Test/Unit/Model/Customer/Plugin/AjaxLoginTest.php +++ b/app/code/Magento/Captcha/Test/Unit/Model/Customer/Plugin/AjaxLoginTest.php @@ -63,7 +63,7 @@ class AjaxLoginTest extends TestCase /** * @var array */ - protected $formIds; + protected $formIds = ['user_login']; /** * @var AjaxLogin @@ -97,7 +97,6 @@ protected function setUp(): void ->method('getCaptcha') ->willReturn($this->captchaMock); - $this->formIds = ['user_login']; $this->serializerMock = $this->createMock(Json::class); $this->model = new AjaxLogin( @@ -194,7 +193,10 @@ public function testAroundExecuteCaptchaIsNotRequired($username, $requestContent $this->captchaMock->expects($this->once())->method('isRequired')->with($username) ->willReturn(false); - $this->captchaMock->expects($this->never())->method('logAttempt')->with($username); + $expectLogAttempt = $requestContent['captcha_form_id'] ?? false; + $this->captchaMock + ->expects($expectLogAttempt ? $this->once() : $this->never()) + ->method('logAttempt')->with($username); $this->captchaMock->expects($this->never())->method('isCorrect'); $closure = function () { diff --git a/app/code/Magento/Developer/Test/Unit/Console/Command/XmlCatalogGenerateCommandTest.php b/app/code/Magento/Developer/Test/Unit/Console/Command/XmlCatalogGenerateCommandTest.php index 919ee0e060468..a9e42cd01f2d5 100644 --- a/app/code/Magento/Developer/Test/Unit/Console/Command/XmlCatalogGenerateCommandTest.php +++ b/app/code/Magento/Developer/Test/Unit/Console/Command/XmlCatalogGenerateCommandTest.php @@ -97,7 +97,7 @@ public function testExecuteVsCodeFormat() ->with( $this->equalTo(['urn:magento:framework:Module/etc/module.xsd' => $fixtureXmlFile]), $this->equalTo('test') - )->willReturn(null); + ); $formats = ['vscode' => $vscodeFormatMock]; $readFactory = $this->createMock(ReadFactory::class); diff --git a/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php b/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php index 5c4a3eb624d3c..39ef03afb83df 100644 --- a/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php +++ b/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php @@ -228,13 +228,10 @@ public function testSetGuest() ->method('removePersistentCookie')->willReturn($this->sessionMock); $this->quoteMock->expects($this->once())->method('isVirtual')->willReturn(false); $this->quoteMock->expects($this->once())->method('getItemsQty')->willReturn(1); - $extensionAttributes = $this->createPartialMock( - CartExtensionInterface::class, - [ - 'setShippingAssignments', - 'getShippingAssignments' - ] - ); + $extensionAttributes = $this->getMockBuilder(CartExtensionInterface::class) + ->addMethods(['getShippingAssignments', 'setShippingAssignments']) + ->disableArgumentCloning() + ->getMockForAbstractClass(); $shippingAssignment = $this->createMock(ShippingAssignmentInterface::class); $extensionAttributes->expects($this->once()) ->method('setShippingAssignments') diff --git a/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php b/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php index 2f8a5a344503c..96c9fbee8834f 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php @@ -59,7 +59,7 @@ protected function setUp(): void $this->stockRegistry = $this->createPartialMock( StockRegistry::class, - ['getStockItem', '__wakeup'] + ['getStockItem'] ); $this->stockItemMock = $this->createPartialMock( \Magento\CatalogInventory\Model\Stock\Item::class, @@ -110,10 +110,14 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri ] ); /** @var Address|MockObject $address */ - $address = $this->createPartialMock( - Address::class, - ['setTotalQty', 'getTotalQty', 'removeItem', 'getQuote'] - ); + $address = $this->getMockBuilder(Address::class) + ->disableOriginalConstructor() + ->disableOriginalClone() + ->disableArgumentCloning() + ->disallowMockingUnknownTypes() + ->onlyMethods(['removeItem', 'getQuote']) + ->addMethods(['setTotalQty', 'getTotalQty']) + ->getMock(); /** @var Product|MockObject $product */ $product = $this->createMock(Product::class); @@ -161,10 +165,13 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri $shippingAssignmentMock->expects($this->exactly(2))->method('getShipping')->willReturn($shipping); $shippingAssignmentMock->expects($this->once())->method('getItems')->willReturn([$quoteItem]); - $total = $this->createPartialMock( - Total::class, - ['setBaseVirtualAmount', 'setVirtualAmount'] - ); + $total = $this->getMockBuilder(Total::class) + ->disableOriginalConstructor() + ->disableOriginalClone() + ->disableArgumentCloning() + ->disallowMockingUnknownTypes() + ->addMethods(['setVirtualAmount', 'setBaseVirtualAmount']) + ->getMock(); $total->expects($this->once())->method('setBaseVirtualAmount')->willReturnSelf(); $total->expects($this->once())->method('setVirtualAmount')->willReturnSelf(); @@ -185,7 +192,10 @@ public function testFetch() ]; $quoteMock = $this->createMock(Quote::class); - $totalMock = $this->createPartialMock(Total::class, ['getSubtotal']); + $totalMock = $this->getMockBuilder(Total::class) + ->addMethods(['getSubtotal']) + ->disableArgumentCloning() + ->getMockForAbstractClass(); $totalMock->expects($this->once())->method('getSubtotal')->willReturn(100); $this->assertEquals($expectedResult, $this->subtotalModel->fetch($quoteMock, $totalMock)); @@ -229,13 +239,14 @@ public function testCollectWithInvalidItems() $address->expects($this->once()) ->method('removeItem') ->with($addressItemId); - $addressItem = $this->createPartialMock( - AddressItem::class, - [ - 'getId', - 'getQuoteItemId' - ] - ); + $addressItem = $this->getMockBuilder(AddressItem::class) + ->disableOriginalConstructor() + ->disableOriginalClone() + ->disableArgumentCloning() + ->disallowMockingUnknownTypes() + ->onlyMethods(['getId']) + ->addMethods(['getQuoteItemId']) + ->getMock(); $addressItem->setAddress($address); $addressItem->method('getId') ->willReturn($addressItemId); diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php index 4cf571d3b6108..e27f6918f1876 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php @@ -49,10 +49,14 @@ class CreditmemoFactoryTest extends TestCase */ protected function setUp(): void { - $this->orderItemMock = $this->createPartialMock( - Item::class, - ['getChildrenItems', 'isDummy', 'getHasChildren', 'getId', 'getParentItemId'] - ); + $this->orderItemMock = $this->getMockBuilder(Item::class) + ->disableOriginalConstructor() + ->disableOriginalClone() + ->disableArgumentCloning() + ->disallowMockingUnknownTypes() + ->onlyMethods(['getChildrenItems', 'isDummy', 'getId', 'getParentItemId']) + ->addMethods(['getHasChildren']) + ->getMock(); $this->orderChildItemOneMock = $this->createPartialMock( Item::class, ['getQtyToRefund', 'getId'] diff --git a/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php b/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php index fc65eabb33d62..58354722d484d 100644 --- a/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php +++ b/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php @@ -50,6 +50,7 @@ protected function setUp(): void 'adj3' => $adj3, 'adj4' => $adj4, ]; + $this->adjustmentsData = $adjustmentsData; /** @var Pool|MockObject $adjustmentPool */ $adjustmentPool = $this->getMockBuilder(Pool::class) @@ -64,14 +65,13 @@ function ($code) use ($adjustmentsData) { return $adjustmentsData[$code]; } ); - $this->adjustmentPool = $adjustmentPool; - $this->adjustmentsData = $adjustmentsData; } /** * @param string[] $adjustments * @param string[] $expectedResult + * * @dataProvider getItemsDataProvider */ public function testGetItems($adjustments, $expectedResult) @@ -92,7 +92,7 @@ public function getItemsDataProvider() [['adj1'], ['adj1']], [['adj4'], ['adj4']], [['adj1', 'adj4'], ['adj1', 'adj4']], - [['adj1', 'adj2', 'adj3', 'adj4'], ['adj3', 'adj1', 'adj2', 'adj4']] + [['adj1', 'adj2', 'adj3', 'adj4'], ['adj3', 'adj1', 'adj2', 'adj4']], ]; } @@ -100,6 +100,7 @@ public function getItemsDataProvider() * @param string[] $adjustments * @param string $code * @param $expectedResult + * * @dataProvider getItemByCodeDataProvider */ public function testGetItemByCode($adjustments, $code, $expectedResult) @@ -108,7 +109,7 @@ public function testGetItemByCode($adjustments, $code, $expectedResult) $item = $collection->getItemByCode($code); - $this->assertEquals($expectedResult, $item->getAdjustmentCode()); + $this->assertEquals($expectedResult, $item->getSortOrder()); } /** @@ -117,11 +118,11 @@ public function testGetItemByCode($adjustments, $code, $expectedResult) public function getItemByCodeDataProvider() { return [ - [['adj1'], 'adj1', $this->adjustmentsData['adj1']], - [['adj1', 'adj2', 'adj3', 'adj4'], 'adj1', $this->adjustmentsData['adj1']], - [['adj1', 'adj2', 'adj3', 'adj4'], 'adj2', $this->adjustmentsData['adj2']], - [['adj1', 'adj2', 'adj3', 'adj4'], 'adj3', $this->adjustmentsData['adj3']], - [['adj1', 'adj2', 'adj3', 'adj4'], 'adj4', $this->adjustmentsData['adj4']], + [['adj1'], 'adj1', 10], + [['adj1', 'adj2', 'adj3', 'adj4'], 'adj1', 10], + [['adj1', 'adj2', 'adj3', 'adj4'], 'adj2', 20], + [['adj1', 'adj2', 'adj3', 'adj4'], 'adj3', 5], + [['adj1', 'adj2', 'adj3', 'adj4'], 'adj4', Pool::DEFAULT_SORT_ORDER], ]; } From 48f5a92a29fbb55d1e687bc1c0ff72075147cf82 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Mon, 27 Jul 2020 09:55:28 +0200 Subject: [PATCH 2/2] process code review comments --- .../Persistent/Test/Unit/Model/QuoteManagerTest.php | 1 - .../Unit/Model/Quote/Address/Total/SubtotalTest.php | 10 ---------- .../Test/Unit/Model/Order/CreditmemoFactoryTest.php | 3 --- .../Pricing/Test/Unit/Adjustment/CollectionTest.php | 4 +--- 4 files changed, 1 insertion(+), 17 deletions(-) diff --git a/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php b/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php index 39ef03afb83df..0c183084edca2 100644 --- a/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php +++ b/app/code/Magento/Persistent/Test/Unit/Model/QuoteManagerTest.php @@ -230,7 +230,6 @@ public function testSetGuest() $this->quoteMock->expects($this->once())->method('getItemsQty')->willReturn(1); $extensionAttributes = $this->getMockBuilder(CartExtensionInterface::class) ->addMethods(['getShippingAssignments', 'setShippingAssignments']) - ->disableArgumentCloning() ->getMockForAbstractClass(); $shippingAssignment = $this->createMock(ShippingAssignmentInterface::class); $extensionAttributes->expects($this->once()) diff --git a/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php b/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php index 96c9fbee8834f..3cc586096d4a0 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/Quote/Address/Total/SubtotalTest.php @@ -112,9 +112,6 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri /** @var Address|MockObject $address */ $address = $this->getMockBuilder(Address::class) ->disableOriginalConstructor() - ->disableOriginalClone() - ->disableArgumentCloning() - ->disallowMockingUnknownTypes() ->onlyMethods(['removeItem', 'getQuote']) ->addMethods(['setTotalQty', 'getTotalQty']) ->getMock(); @@ -167,9 +164,6 @@ public function testCollect($price, $originalPrice, $itemHasParent, $expectedPri $total = $this->getMockBuilder(Total::class) ->disableOriginalConstructor() - ->disableOriginalClone() - ->disableArgumentCloning() - ->disallowMockingUnknownTypes() ->addMethods(['setVirtualAmount', 'setBaseVirtualAmount']) ->getMock(); $total->expects($this->once())->method('setBaseVirtualAmount')->willReturnSelf(); @@ -194,7 +188,6 @@ public function testFetch() $quoteMock = $this->createMock(Quote::class); $totalMock = $this->getMockBuilder(Total::class) ->addMethods(['getSubtotal']) - ->disableArgumentCloning() ->getMockForAbstractClass(); $totalMock->expects($this->once())->method('getSubtotal')->willReturn(100); @@ -241,9 +234,6 @@ public function testCollectWithInvalidItems() ->with($addressItemId); $addressItem = $this->getMockBuilder(AddressItem::class) ->disableOriginalConstructor() - ->disableOriginalClone() - ->disableArgumentCloning() - ->disallowMockingUnknownTypes() ->onlyMethods(['getId']) ->addMethods(['getQuoteItemId']) ->getMock(); diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php index e27f6918f1876..67f1931cf7bd1 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/CreditmemoFactoryTest.php @@ -51,9 +51,6 @@ protected function setUp(): void { $this->orderItemMock = $this->getMockBuilder(Item::class) ->disableOriginalConstructor() - ->disableOriginalClone() - ->disableArgumentCloning() - ->disallowMockingUnknownTypes() ->onlyMethods(['getChildrenItems', 'isDummy', 'getId', 'getParentItemId']) ->addMethods(['getHasChildren']) ->getMock(); diff --git a/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php b/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php index 58354722d484d..ae0e006de59a2 100644 --- a/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php +++ b/lib/internal/Magento/Framework/Pricing/Test/Unit/Adjustment/CollectionTest.php @@ -71,7 +71,6 @@ function ($code) use ($adjustmentsData) { /** * @param string[] $adjustments * @param string[] $expectedResult - * * @dataProvider getItemsDataProvider */ public function testGetItems($adjustments, $expectedResult) @@ -92,7 +91,7 @@ public function getItemsDataProvider() [['adj1'], ['adj1']], [['adj4'], ['adj4']], [['adj1', 'adj4'], ['adj1', 'adj4']], - [['adj1', 'adj2', 'adj3', 'adj4'], ['adj3', 'adj1', 'adj2', 'adj4']], + [['adj1', 'adj2', 'adj3', 'adj4'], ['adj3', 'adj1', 'adj2', 'adj4']] ]; } @@ -100,7 +99,6 @@ public function getItemsDataProvider() * @param string[] $adjustments * @param string $code * @param $expectedResult - * * @dataProvider getItemByCodeDataProvider */ public function testGetItemByCode($adjustments, $code, $expectedResult)