Skip to content
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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bubasuma
Copy link

@bubasuma bubasuma commented May 25, 2022

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

  • @magentoAppIsolation -> AppIsolation
  • @magentoDbIsolation -> DbIsolation
  • @magentoDataFixtureBeforeTransaction ->DataFixtureBeforeTransaction
  • @magentoDataFixture -> DataFixture (1)
  • @magentoApiDataFixture -> DataFixture (1)
  • @magentoIndexerDimensionMode-> IndexerDimensionMode
  • @magentoComponentsDir -> ComponentsDir
  • @magentoAppArea -> AppArea
  • @magentoCache -> Cache
  • @magentoAdminConfigFixture -> Config (2)
  • @magentoConfigFixture -> Config (2)

(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

  • Best time to make such changes as Parameterized Fixture is not released yet
  • Fixture classes can be imported and referenced using constant (Fixture::class)
  • PHP Native syntax
  • Easy to maintain and extend

Cons

  • Require PHP >= 8.0 to run tests

Thumbs up 👍 if you like this proposal

@fooman
Copy link

fooman commented Jun 30, 2022

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
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

3 participants