MFTF test packaging proposal

[okolesnyk]

Problem

Tests are dead code on production env.

Solution

Exclude tests from magento2-module package and introduce new type of packages with Tests only
There is going to be new modules which will have Functional Tests only.
Example:
CatalogAdminUi - will have Magento Admin UI logic inside
CatalogAdminUiFunctionalTest - will have Automated Test Coverage for Catalog Magento Admin UI module

Requested Reviewers

@paliarush
@maghamed
@buskamuza
@antonkril

[vkublytskyi]

For sure, code that may be executed is a subject of attack but in case of MFTF module contains only XML configuration so no executable that may be used somehow while MFTF itself is not installed.

Another question that deserves explicit discussion is what is the production environment for Magento. I totally agree that running instance of Magento that serve real merchant customers should not contain any executable test code. But are composer packages that we produce production code or not? With removing our tests from packages we increase the complexity of system integrators and vendors workflow and internal and external CI infrastructures. It also leads to controversial decisions in release management and packaging.

Alternatively, we may consider a possibility to provide instruments to remove test code (including from any possible 3rd party extensions and SI customizations) as part of operations during deploying Magento. This may be done as CLI command which may be part of enabling the production mode. Or it may be done by Composer plugin that will remove test code if --no-dev flag is used for composer install or composer update commands. In addition, we may detect available test code and show a warning message in the admin panel to clearly communicate potential security issue and need to perform actions on the server side.

[okolesnyk]

@melnikovi Yes, I will investigate tooling for that. Will update document afterwards.

[buskamuza]

type: "magento2-test-module" - will this information be used anywhere or can we use just standard Composer type library?

[piotrekkaminski]

functional-tests (instead of test?)

[okolesnyk]

Yes. This proposal is about packaging Functional tests only. And they shouldn't be distributed with other test types together.

[ravmenon]

Overall, I am in favor of this proposal. It aligns well with the the best practices mentioned here: https://sormuras.github.io/blog/2018-09-11-testing-in-the-modular-world.html

[okolesnyk]

@ravmenon Please take a look at files section and you will see that proposal was changed drastically. So some of your comments goes to outdated part.

[buskamuza]

It looks odd to include project in a project. Maybe this metapackage should not be called project? And someone can include it in the "Magento project" or to create an empty "project" for tests and include this metapackage. Could you please also include more details what do you expect in the metapackage? Does it include all the frameworks?

[okolesnyk]

Yes, that is typo. It meant to be magento/functional-test-product-community-edition

[buskamuza]

Also, documentation should explicitly state that Web Setup Wizard does not support testing packages and it's intentional (because it's considered to be used in production). Please confirm with PO and validate that this is real behavior (when implementing).

[lbajsarowicz]

Performance:

• MFTF coverage grows drastically, which side effect is growing deployment time for staging / production environment

[lbajsarowicz]

I'm not sure if I'm missing something, but...
I'd take another approach. Each module having it's FunctionalTest dependency in it's composer.json as a -dev requirement. So that doint composer install --no-dev just does not install the tests.

In practice - each module needs tests only for dev environment, so this dependency would be reasonable.

[okolesnyk]

Adding here discussion from Slack channel to not loose it.

Kristof Ringleff, Fooman [Jun 20th at 6:50 PM]
@okolesnyk thanks for sharing. My immediate thoughts are:

1. the functional test metapackage should not live in the require-dev section of
   magento/project-community-edition
   but rather in the require-dev section of magento/product-community-edition

I believe the sooner Magento understands that magento/project-community-edition is merely a suggested starting template for a project the better. (it's like using eject on a node package, from there you are on your own and Magento no longer is able to easily update it).

2. Looking at my own tests so far I am not sure if
   CatalogAdminUiFunctionalTest
   is too granular. Usually with my acceptance tests I need to configure something, checkout, look again in the admin. I believe a lot of tests would be like that (otherwise it could be more easily covered with other types of tests). So if I were to split the actual tests into multiple packages I end up with nothing that actually tests meaningful up until I combine them all again anyway. In which case it would be easier to just have CatalogFunctionalTest. But maybe for now if this is proposal is for Magento Core only and extension devs can decide to follow the split or keep them in one that would work for me.
   (I guess I am generally not quite sure how the service isolation is going to work with tests)

3. There is a similar challenge around integration tests. Any thoughts on how these would work alongside this proposal?
   4 replies

Alex Kolesnyk [1 day ago]
Hi @fooman

1. Agree with you. I think we should add all test packages in the require-dev block of magento/product-community-edition.
   But the thing is that we will have a possibility to install Magento without tests and I wanted to have possibility to do the same with tests.
   Let’s imagine you have Magento Instance somewhere out of local env and you what to run and write Functional tests only. Yes, you can composer install same package but you will have unnecessary Magento application code.
   What do you think if we also will generate magento/functional-tests-community-edition metapackage which will include under require only test packages ? (edited)

2. That’s a good point. The idea why I invented :rolling_on_the_floor_laughing: this naming convention {ModuleName}FunctionalTest is very simple. Currently before generating all tests MFTF makes request to Magento WebAPI asking a list of enabled modules and component load order. This gives MFTF a possibility to generate only tests for enabled modules and using load order we know in what order we should merge all XML entities.
   If we will make possible to create a Single Test Module for some group of Magento modules we need to figure out how to specify in TestModule what Modules must be enabled in Magento. For this I created an issue under MFTF repo where you can put your thoughts [Discussion] MFTF tests in separate Magento modules magento2-functional-testing-framework#373
   GitHub
   [Discussion] MFTF tests in separate Magento modules · Issue Integration test data fixture change proposal #373 · magento/magento2-functional-testing-framework
   Goals Discuss changes related to MFTF test packaging proposal #172 Build backlog to support proposed Module structure for MFTF tests

3. No, we don’t have it for Integration tests yet, but out of this proposal right after we approve it I think it will be much easier to build something similar for Integration tests and maybe for all other test types.

[xmav]

We might want to move "magento/magento2-functional-testing-framework" from 'require-dev' to new meta package with tests as well

Edited:
In order to support existing mftf tests in 3rd party modules we should keep it

[buskamuza]

@lbajsarowicz, dev dependencies are supported in the root composer.json only.

[buskamuza]

the functional test metapackage should not live in the require-dev section of
magento/project-community-edition
but rather in the require-dev section of magento/product-community-edition

Same. It will not work. require-dev is supported on the project level only.

[okolesnyk]

@buskamuza @melnikovi @paliarush @maghamed Do I need to do something with this proposal? Or we can merge it and I will proceed with implementation tasks and implementation itself?

[fooman]

@buskamuza sorry missed this discussion and your comment. Keeping the testing metapackage out of the root composer.json require-dev section was exactly my point. If someone chooses to use the same tools as Magento for their development - great. It's only a
composer require --dev magento/functional-metapackage away.

[buskamuza]

magento/project-community-edition is merely a suggested starting template for a project

Exactly. So we're adding the testing metapacakge to reqiuire-dev section of the suggested project template. If somebody doesn't want to use it, they can modify the composer.json. As soon as the project owner creates the project, composer.json is not under Magento's control anymore.
But it's a good place to show what exists natively.
In any case, I was just noticing that we can't use require-dev section in the "product" metapackage because it will not work. From there, we can either include or exclude tests from the project.

[fooman]

@buskamuza I was hoping that this proposal here #157 was going ahead.