-
Notifications
You must be signed in to change notification settings - Fork 199
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
Test directory is inconstant #268
Comments
Hi @mikeymike Thank you for posting this issue with good details. Internal ticket MAGETWO-67324 to solve it. |
No problem @victor-v-rad .... I'd submit a PR but I'm unsure of the testing procedure for the package. Do the tests run through a core Magento test suite ? |
Understood, I'm familiar with how this is generally done within community packages although I'm unsure of why this wouldn't be seen as an ordinary module of Magento 2? Is there any other context this would be run in other than being a module ? Either way do you think the better solution is to change the check inside Magento ? It should be a relatively simple fix with no real impact on anything else afaik. |
You could just delete 'tests' folder if not used. |
Yup, we're doing it on a post install script with composer. Thanks though 👍 |
This looks like it's solved just awaiting release 👍 🎉 |
Generally M2 modules conform to a pretty rigid structure (albeit not enforced in any way) and one part of this is the test directory.
All modules have them defined as
Test
and by calling ittests
here you're causing the di compile command to compile the tests too.Here's the line that makes it skip the
Test
directory https://github.com/magento/magento2/blob/develop/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php#L228-L230Note that you want actually see any errors under normal conditions but because we've replaced the ancient version of PHPUnit used by Magento2 with PHPUnit 6 we've also lost the
PHPUnit_Framework_TestCase
class so the DI compile errors for us.It's clear anyway that tests shouldn't be compiled so I think we should rename the directory to conform 😄
The text was updated successfully, but these errors were encountered: