search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.54k stars 9.32k forks source link

Files mapped to dev/tests by "required" composer packages #8971

Open korostii opened 7 years ago

korostii commented 7 years ago

The composer.json file distributed with the application contains separate sections "require" and "require-dev" sections. It is commonly expected that "require" section only contains dependencies necessary for production use, and not tests, tools used for development etc.

Preconditions

  1. Magento 2.1.5 CE from https://magento.com/tech-resources/download

Steps to reproduce

  1. Run "composer install --no-dev"

Expected result

  1. No folder "dev/" in the application root or empty "dev" folder.

Actual result

  1. Some files inside under dev/

To be precise, it is the magento2-base package maps quite a lot of its files into the dev folder, namely subfolders "dev/tools", "dev/travis" and "dev/tests/". These do not seem to be an integral part of the application itself, and as such must be separate into a package included inside the "require-dev" section of the composer. Accordingly, it would also be nice to see the "dev" folder added into the sample ".gitignore" file distributed with the package available from https://magento.com/tech-resources/download.

korostii commented 7 years ago

See also #5262, #4162, Internal ticket MAGETWO-52095

magento-engcom-team commented 7 years ago

@korostii, thank you for your report. We've created internal ticket(s) MAGETWO-52095 to track progress on the issue.

orlangur commented 7 years ago

These do not seem to be an integral part of the application itself, and as such must be separate into a package included inside the "require-dev" section of the composer.

Disagree.

According to my understanding:

  1. dev/ folder must be dissolved as much as possible among modules, not just unit tests deserve to be modular
  2. .gitattributes may be used to get rid of tests in Composer packages but as far as I remember PHPUnit configurations were intentionally adopted to be able to run tests from vendor/

It is commonly expected that "require" section only contains dependencies necessary for production use, and not tests, tools used for development etc.

Such expectation is wrong: https://getcomposer.org/doc/03-cli.md#depends-why- require-dev section has nothing to do with dev/ folder, only related to packages.

@magento-engcom-team I propose to close this issue as won't fix. Something like https://www.reddit.com/r/PHP/comments/2jzp6k/i_dont_need_your_tests_in_my_production/#bottom-comments may be reported as a low priority improvement but then all testing scenarios needs to be considered - prefer-dist, prefer-source - so that it is still possible to test your extension together with all core modules. Unrelated code which allows to run tests from vendor should be cleaned up.

korostii commented 7 years ago

Personally I do not insist on any particular technical solution to the issue and it's clearly low priority. But there absolutely should be a way to skip tests, travis and other stuff which is not required for running in production, and it should be possibly to do via a simple CLI one-liner.

IMHO even approach similar to http://devdocs.magento.com/guides/v2.0/install-gde/install/cli/install-cli-sample-data-other.html would be just fine.

m2-assistant[bot] commented 4 years ago

Hi @engcom-Bravo. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

magento-engcom-team commented 4 years ago

@engcom-Bravo Thank you for verifying the issue.

Unfortunately, not enough information was provided to acknowledge ticket. Please consider adding the following:

Once all required information is added, please add label "Issue: Confirmed" again. Thanks!

magento-engcom-team commented 4 years ago

:white_check_mark: Confirmed by @engcom-Bravo Thank you for verifying the issue. Based on the provided information internal tickets MC-31127 were created

Issue Available: @engcom-Bravo, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

m2-assistant[bot] commented 4 years ago

Hi @korostii. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Thank you for your contributions.

hostep commented 3 years ago

This should still happen one day...

There is a proposal for a fix in https://github.com/magento/magento2/issues/5262 but no idea if this is the best fitted solution:

I think most of these files are getting into place because of the mapping in the composer.json file of the magento2-base module. It would be great if those mappings can get split of into a mapping for development purposes and one for all environments. My suggestion would be to add a map-dev to the extra section in the composer.json similar as the normal map section and update the magento-composer-installer project to support this and only install the map-dev files when you run composer without the --no-dev flag.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

hostep commented 3 years ago

Is still relevant in my opinion even though it will be considered low priority

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

fredden commented 3 years ago

This is still an issue that requires resolution.

Manju-crz commented 1 year ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 1 year ago

Hi @Manju-crz. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @Manju-crz, here is your Magento Instance: https://38fbdc1296d3cbd2532411bbf0e85736.instances-prod.magento-community.engineering Admin access: https://38fbdc1296d3cbd2532411bbf0e85736.instances-prod.magento-community.engineering/admin_0833 Login: 26c45822 Password: 4cbf9965bfd4