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

Integration test data fixture change proposal #373

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bubasuma
Copy link

Problem

Actually there is no formal way to customize existing data fixture directly from the test case which leads to duplicated data fixtures.

Solution

Extend the format of data fixture annotation to support a second parameter which will be injected to the data fixture file to customize the fixture for a specific test scenario.

Requested Reviewers

@buskamuza

Copy link
Contributor

@buskamuza buskamuza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, looks good and brings good value. Thanks!

design-documents/testing/integration/data-fixtures.md Outdated Show resolved Hide resolved
@mykhailomatiola
Copy link

Interesting idea, but it has several problems:

  1. Works well with a simple fixture (simple product, cms page), doesn't work well with complex fixtures(quote, order, bundle product)
    For example if we create some basic factory for quote some other basic types should be passed into it(product as quote item, customer if this is not a guest quote and other...). But if we need some specific quote (with bundle product with specific options) but we have a factory for simple bundle product, json for such quote became huge because wee need to create such product inside annotation via json.
  2. It is difficult to transfer data from other fixtures. You have to do everything inside (duplication of creation) or transfer static data from the outside, which is not always possible, since most entity relations work through the IDs.
    For example product with category. You have already created category in another fixture and you need to path category id into your fixture.
  3. In case of a big fixture, json becomes unreadable, the name of the fixture isn't informative, and we just transfer static data from the fixture to test annotation.
    For example If you analyze mentioned above the simple quote constructor should be name something like 'simple_quote'. But if we need some specific quote the name of basic quote will be the same. We just need to add huge json to annotation. After this we need analyze all json to understand difference between this and simple quote. Currently, to understand a quote type simply enough read a name.
  4. This fixtures is untyped, out of the box it is not clear how to use it.
  5. Most of all simple fixtures already created.

@bubasuma
Copy link
Author

bubasuma commented Apr 28, 2020

@mykhailomatiola
This proposal is not focused on how you create fixtures data or how you create complex entities like quote, bundle product ...
The main idea is to make fixtures data customizable from the test.
The factory class in the example is not the main purpose of this proposal but simply a code isolation to make the data fixture file more readable.
I agree with you on the issue-3. But most of the time you will not need to customize all data. That's a rare situation and probably would be better in that case to create a new fixture file. I believe the data fixture should have already default data like in the example.

In case of a big fixture, json becomes unreadable

@dhorytskyi
Copy link
Contributor

@bubasuma
Copy link
Author

bubasuma commented Apr 28, 2020

@dhorytskyi
I believe in this case the JSON will be passed as the function parameter like the following

/**
     * @magentoDataFixture roleDataFixture {"role":{"name":"admin_role"}, "someVar": "value"}
     */
    public function testGetRole()
    {
    }

    public static function roleDataFixture(array $data)
    {
    }

Or we could unpack the data array across function parameters.

    public static function roleDataFixture(array $role, string $someVar)
    {
    }

But you will rarely need to customize function data fixture as they're written for your test only.

@bubasuma
Copy link
Author

bubasuma commented Jun 1, 2020

@buskamuza @dhorytskyi
I have updated the proposal with the following changes:

  • Replace JSON format with a data provider method that returns an array of data which will be used to customize fixtures and resolve dependencies between fixtures
  • Add ability to define and resolve dependencies between fixtures
  • Extend overrides.xml (Integration tests extensibility) to support fixture data customization
  • Add an alternative solution to replace file based fixtures

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

Successfully merging this pull request may close these issues.

5 participants