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

Decomposition of dev/tests testsuites across corresponding modules #28012

Open
bartoszkubicki opened this issue Apr 28, 2020 · 10 comments · May be fixed by #28733
Open

Decomposition of dev/tests testsuites across corresponding modules #28012

bartoszkubicki opened this issue Apr 28, 2020 · 10 comments · May be fixed by #28733
Assignees
Labels
feature request Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Priority: P3 May be fixed according to the position in the backlog. Progress: PR in progress Reported on 2.3.x Indicates original Magento version for the Issue report. Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@bartoszkubicki
Copy link

Summary (*)

Magento 2.3 or 2.4. There are a lot of tests inside dev/tests directory, which violates modularity of platform, because while uninstalling (I mean completely removing) package from vendor tests stays in dev/tests. Wouldn't be better to spread them across corresponding modules? What if, I would like to uninstall module Magento_Wishlist and still run all integration tests? I still will have its tests in dev/tests/integration/testsuite/Magento/Wishlist, which won't work if I remove composer package with physical code.

Examples (*)

Test inside dev/tests/integration/testsuite/Magento/*.

Proposed solution

Copy-paste modules tests - I could start with integration ones into corresponding modules, so testsuite is decoupled from fixed modules set.

@bartoszkubicki bartoszkubicki added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Apr 28, 2020
@m2-assistant
Copy link

m2-assistant bot commented Apr 28, 2020

Hi @bartoszkubicki. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

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

@bartoszkubicki do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Apr 28, 2020
@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Apr 28, 2020

Related to #21465 and magento/inventory#2876, I see for MSI integration tests were already moved to the module directories.

@bartoszkubicki
Copy link
Author

@ihor-sviziev is there a chance that PRs transporting (and by the way cleaning up formatting) is going to be accepted?

@ihor-sviziev
Copy link
Contributor

AFAIK there is quite big dependency mess in the integration tests, so basically a lot of tests should be improved, this is definitely not a small part.

Fixing fixture paths could be the first step forward, similar to magento/inventory#2931. Such PR could be accepted in any patch release. But huge test refactoring - not sure if they could be accepted before 2.5.0.

@ihor-sviziev ihor-sviziev added the Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. label Apr 28, 2020
@bartoszkubicki
Copy link
Author

bartoszkubicki commented Apr 28, 2020

@ihor-sviziev my idea is not to do all job in one PR, but for example, one PR per module having a few tests - examples - Magento_AdminNotification - 5 integration tests + 1 fixture. More problematic of course will be refactoring of bigger modules like Magento_Catalog, so smaller ones would be good starting point.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Apr 28, 2020

@bartoszkubicki personally I'm ok with it, but after migration to phpunit 9 & php7.4, just to prevent fixing a lot of conflicts and not to loose any change

@bartoszkubicki
Copy link
Author

bartoszkubicki commented Apr 28, 2020

phpunit 9 & php7.4

For which release is this planned? I don't think, just moving classes will do any harm. By slight refactor I mean optimizing imports, nothing really changing logic of test. My proposal is to preapre small PR, for smallest module I can find - I think it is Magento_Rss - it has only one test, and we would have kind of prototype, how this migration could look like.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Apr 28, 2020 via email

@bartoszkubicki
Copy link
Author

@ihor-sviziev PR is ready #28053

@sdzhepa sdzhepa linked a pull request Apr 29, 2020 that will close this issue
4 tasks
@ghost ghost assigned bartoszkubicki and unassigned bartoszkubicki Jun 15, 2020
@ghost ghost added the Priority: P3 May be fixed according to the position in the backlog. label Jun 16, 2020
@m2-community-project m2-community-project bot added Progress: PR Created Indicates that Pull Request has been created to fix issue and removed Progress: PR in progress labels Sep 23, 2020
@m2-community-project m2-community-project bot added Progress: PR Created Indicates that Pull Request has been created to fix issue and removed Progress: PR Created Indicates that Pull Request has been created to fix issue labels Sep 24, 2020
@magento-engcom-team magento-engcom-team added the Reported on 2.3.x Indicates original Magento version for the Issue report. label Nov 13, 2020
@m2-community-project m2-community-project bot added Progress: PR in progress and removed Progress: PR Created Indicates that Pull Request has been created to fix issue labels Nov 13, 2020
@engcom-Bravo
Copy link
Contributor

Hi @bartoszkubicki,

Thank you for reporting and collaboration..

Verified the behavior on Magento 2.4-develop instance as this is existing behavior of Magento, Hence We are considering this as feature request.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Priority: P3 May be fixed according to the position in the backlog. Progress: PR in progress Reported on 2.3.x Indicates original Magento version for the Issue report. Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Status: Pull Request in Progress
4 participants