magento / architecture

A place where Magento architectural discussions happen
274 stars 154 forks source link

Introduce an alternative way to define fixtures using PHP8 Attributes #509

Open bubasuma opened 2 years ago

bubasuma commented 2 years ago

Problem

Recently, we introduced parameterized fixture for integration and api-functional test that accepts parameters directly from @magentoDataFixture annotation.

We enhanced @magentoDataFixture annotation format to support additional information that contains the parameters and the alias of a fixture. And we introduced a new annotation @magentoDataFixtureDataProvider for advanced fixture configuration.

However, with the current implementation, fixture parameters have to be passed in JSON format which is hard to read and maintain. PHP 8.0 introduced a new feature (Attributes) that we can utilize to generate fixtures instead of DocBlock annotations which will enable us to use PHP build in syntax and increase the code readability.

Example

class AddSimpleProductToCartSingleMutationTest extends GraphQlAbstract
{
    /**
     * @magentoApiDataFixture Magento\Catalog\Test\Fixture\Product as:product1
     * @magentoApiDataFixture Magento\Catalog\Test\Fixture\Product as:product2
     * @magentoApiDataFixture Magento\Catalog\Test\Fixture\Product as:product3
     * @magentoApiDataFixture Magento\Quote\Test\Fixture\GuestCart as:cart
     * @magentoApiDataFixture Magento\Quote\Test\Fixture\AddProductToCart as:cartItem1
     * @magentoApiDataFixture Magento\Quote\Test\Fixture\AddProductToCart as:cartItem2
     * @magentoDataFixtureDataProvider {"cartItem1":{"cart_id":"$cart.id$","product_id":"$product1.id$","qty":1}}
     * @magentoDataFixtureDataProvider {"cartItem2":{"cart_id":"$cart.id$","product_id":"$product2.id$","qty":1}}
     */
    public function testAddMultipleProductsToNotEmptyCart(): void
    {
        $product1 = $this->fixtures->get('product1');
        $product2 = $this->fixtures->get('product2');
        $product3 = $this->fixtures->get('product3');
        $cart = $this->fixtures->get('cart');
        //...
    }
}

Solution

PHP 8.0 introduced a new feature (Attributes) that we can utilize to generate fixtures instead of DocBlock annotations.

Implementation

Create PHP attribute equivalent for all existing DocBlock annotations used in integration and functional tests

(1) Both magentoDataFixture and magentoApiDataFixture can be replaced with DataFixture attribute. Currently we use magentoDataFixture in integration tests and magentoApiDataFixture in functional api tests (WebAPI). Both executes data fixtures except that magentoApiDataFixture does not trigger db transaction (db isolation) by default (without explicitly enabling db isolation with @magentoDbIsolation) whereas magentoDataFixture does.

All it takes to make this work is to replace \Magento\TestFramework\Annotation\DataFixture with \Magento\TestFramework\Annotation\ApiDataFixture in WebapiDocBlock and override \Magento\TestFramework\Annotation\AbstractDataFixture::getDbIsolationState in \Magento\TestFramework\Annotation\ApiDataFixture (ApiDataFixture extends DataFixture which also extends AbstractDataFixture) to return [disabled] instead of NULL in case the db isolation is not explicitly defined. This will prevent the transaction from auto starting in \Magento\TestFramework\Annotation\DataFixture::startTestTransactionRequest unless explicitly enabled with @magentoDbIsolation.

(2) Both magentoAdminConfigFixture and magentoConfigFixture can be replaced with Config attribute.

Example:

namespace Magento\TestFramework\Fixture;

use Attribute;

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
class DataFixture
{
    /**
     * @param string $type
     * @param array $data
     * @param string|null $as
     */
    public function __construct(
        public string $type,
        public array $data = [],
        public ?string $as = null
    ) {
    }
}
#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
class Config
{
    /**
     * @param string $path
     * @param mixed $value
     * @param string $scopeType
     * @param string|null $scopeValue
     */
    public function __construct(
        public string $path,
        public mixed $value,
        public string $scopeType = ScopeConfigInterface::SCOPE_TYPE_DEFAULT,
        public ?string $scopeValue = null
    ) {
    }
}
class AddSimpleProductToCartSingleMutationTest extends GraphQlAbstract
{
    #[
        Config(Configuration::XML_PATH_SHOW_OUT_OF_STOCK, 1, ScopeInterface::SCOPE_STORE, 'default'),
        DataFixture(ProductFixture::class, as: 'product1'),
        DataFixture(ProductFixture::class, as: 'product2'),
        DataFixture(ProductFixture::class, as: 'product3'),
        DataFixture(GuestCartFixture::class, as: 'cart'),
        DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$product1.id$']),
        DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$product2.id$']),
    ]
    public function testAddMultipleProductsToNotEmptyCart(): void
    {
        $product1 = $this->fixtures->get('product1');
        $product2 = $this->fixtures->get('product2');
        $product3 = $this->fixtures->get('product3');
        $cart = $this->fixtures->get('cart');
        //...
    }
}

Pros

Thumbs up 👍 if you like this proposal

fooman commented 2 years ago

I am all for improvements for the testing framework's abilities. One concern which I will voice here in the hope of it being considered. If/and when the new fixtures get introduced can the existing fixture files (like for example these https://github.com/magento/magento2/tree/2.4.4/dev/tests/integration/testsuite/Magento/Sales/_files) please be deprecated rather than getting removed immediately.

Reason being it will be quite a few years until php 7.x will no longer be in use in the Magento ecosystem and we are actively using the current fixtures in our testing. While Adobe might be able to move on quickly we as extension developers try to support older versions while they are still popular.

fascinosum commented 2 years ago

This is a step in the right direction as it makes it much easier to use configurable fixtures