Fix #10438: Potential error on order edit page when address has extension attributes #11787
Conversation
|
|
||
| $object = $this->adminOrderCreate->applyCoupon($couponCode); | ||
| $this->assertEquals($this->adminOrderCreate, $object); | ||
| self::assertEquals($this->adminOrderCreate, $object); |
There was a problem hiding this comment.
$this->assert* must be used, change all occurrences.
There was a problem hiding this comment.
@orlangur, assert* it's a static function. Also, I didn't find any requirements about $this->...* usage instead self::... for tests.
There was a problem hiding this comment.
Easy, it's a recommendation of PHPUnit author, we should not rely on 15 years old things from Java: sebastianbergmann/phpunit#1914 (comment)
Maybe I'll implement static test enforcing this someday but till then please keep things consistent, most of calls in Magento codebase are $this->assert currently.
There was a problem hiding this comment.
As I see, it's just a recommendation. We are using self::assert more than 1 year in the tests.
Another point, it's calling static methods as non-static could be changed in future versions, the same we have got for PHP 5.6 for calling non-static as static - http://php.net/manual/en/migration56.deprecated.php
There was a problem hiding this comment.
Of course you can use self:: technically but it's lame.
31352 occurrences of $this->assert vs 296 of self::assert, hm, not too much tests written during more than a year :)
There is no point in continuing discussion but now I see that a test enforcing best practices is really needed.
magento-engcom-team
added
2.2.x
bugfix
Issue: Cannot Reproduce
YevSent
removed
the
Issue: Cannot Reproduce
| @@ -296,7 +304,8 @@ public function __construct( | |||
| \Magento\Sales\Api\OrderManagementInterface $orderManagement, | |||
| \Magento\Quote\Model\QuoteFactory $quoteFactory, | |||
| array $data = [], | |||
| \Magento\Framework\Serialize\Serializer\Json $serializer = null | |||
| \Magento\Framework\Serialize\Serializer\Json $serializer = null, | |||
| ExtensibleDataObjectConverter $dataObjectConverter = null | |||
There was a problem hiding this comment.
Please, update docblock accordingly with the parameters that you accept.
- Changed shipping and billing address comparision - Refactored related tests
…dress has extension attributes #11787
magento-engcom-team
added
the
Fixed in 2.2.x
Fixed Issues
Contribution checklist