Integration testssuite decomposition - Rss and AdminNotification

[bartoszkubicki]

Description (*)

All idea comes from this issue. @ihor-sviziev I took apart of Magento_Rss, Magento_AdminNotification also, as there is test here which contains also file data provider. Generally, it was very easy to move the test and locally all tests worked the same way. I don't see any problem in moving, and refactoring tests in similar way (optimizing imports, some return types, types hints)

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes Decomposition of dev/tests testsuites across corresponding modules #28012

Manual testing scenarios (*)

1. Just run moved integration tests. If everything still works and tests pass it means that they were successfully decomposed from testsuite

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

** This is cleaned up port of #28053**

[m2-assistant]

Hi @bartoszkubicki. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

[ihor-sviziev]

@magento run all tests

[ihor-sviziev]

✔ Approved.

Failing functional tests looks not related to changes form this PR.

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7688 has been created to process this Pull Request

[engcom-Alfa]

QA not applicable

Pull Request state was updated. Re-review required.

[engcom-Charlie]

Need additional discussion to approve such changes, put on hold for now.

[ihor-sviziev]

@engcom-Charlie is there any updates about this PR? Can we move it forward?

[engcom-Charlie]

Hi @naydav, is there any update?

[sidolov]

Hi @ihor-sviziev , the location of the tests in Magento is under discussion right now. We have the same approach for our bundled extensions, like MSI, Adobe Stock integration, etc. I'll keep you updated and will leave the feedback once we get the decision. It would be great to share with us pros and cons of such approach. Thank you!

[ihor-sviziev]

@sidolov
In general I think it's a good idea to have all tests in one place for the module, I would prefer also moving all js unit, web api tests and others, but unit tests we already have per module, next closest to unit - integration tests.

pros - having all tests for the module in one place.
cons - From the tests perspective - quite frequently integration tests depends on other modules, so technically "dev" dependencies should be declared in the require-dev section of the module's composer (not sure if it's actually needed)

[sidolov]

@ihor-sviziev agree with you on dependencies declaration concern. Yeah, require-dev section should resolve the problem and avoid strict dependencies for production environments.

[bartoszkubicki]

@ihor-sviziev @sidolov I think in most of cases if integration test have dependency, also tested code has similar dependency (for example integrations tests for checkout needs customer module, but real code needs them also). I don't see any serious problem here.

This PR try to show the way to resolve a problem going a little bit opposite way. Let's say we want to remove some modules, that we don't use and it can be done nicely composer (replace with *). This needs well declared dependencies (no hidden) and real modularity (also on test level). However, as long as integration tests are away from modules and are part of project, despite we don't use modules, we make integration tests suite useless as a whole. We have to keep all modules or just don't run them (or manually remove folders for modules we no longer have, what is of course terrible idea, if for example we have to restore module for some reason).

[gabrieldagama]

Risk set to low since tests shouldn't affect other areas.

[sivaschenko]

Pending discussion and approve of moving integration tests to modules by the Dev Guild

[ihor-sviziev]

@sivaschenko any updates on this? One month already passed...

Also for some reason this PR was moved again to "review". Does it mean we can approve and merge it?
/CC @sidolov

[ihor-sviziev]

Seems like it was mistake. Moved back to correct status

[sivaschenko]

Completing approval for this pull request can be delayed as we are processing PRs in priority order

[bartoszkubicki]

@ihor-sviziev, @sivaschenko what is the status of this PR?