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

Add PHPUnit missing in dependencies #267

Merged
merged 7 commits into from
Nov 16, 2018
Merged

Add PHPUnit missing in dependencies #267

merged 7 commits into from
Nov 16, 2018

Conversation

lbajsarowicz
Copy link
Contributor

Description

Basically in composer.json there's a PHPUnit dependency missing, so that bin/phpunit-checks is unable to run if phpunit is not installed globally.

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/verification tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)
  • Changes to Framework doesn't have backward incompatible changes for tests or have related Pull Request with fixes to tests

Copy link
Member

@okolesnyk okolesnyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lbajsarowicz composer.lock should be updated also.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 57.411% when pulling d24ac0a on M2Coach:missing-phpunit into a9e04a7 on magento:develop.

@coveralls
Copy link

coveralls commented Nov 5, 2018

Coverage Status

Coverage remained the same at 57.392% when pulling 5a4e2ef on M2Coach:missing-phpunit into 7136da4 on magento:develop.

@lbajsarowicz
Copy link
Contributor Author

@okolesnyk composer.lock commited.

composer.json Outdated Show resolved Hide resolved
@lbajsarowicz
Copy link
Contributor Author

@aljcalandra just want to stay compliant with Magento 2 core. Is there any reason to introduce dependency that does not stay in line with core?

@okolesnyk
Copy link
Member

@lbajsarowicz
MFTF can also be used as a Stand-alone tool, so we think it is ok first of all to support what Magento supports and adding an additional possible version of PHPUnit. Also we do this for possible future updates of PHPUnit dependency in Magento.

@lbajsarowicz
Copy link
Contributor Author

@okolesnyk As far as I see you still want to have compatibility with PHP 7.0, but
screenshot 2018-11-14 at 06 18 42

I ran composer update from PHP 7.0 to keep composer.lock compliant with PHP 7.0, so it has not changed (that's why it was not commited)

@okolesnyk
Copy link
Member

@lbajsarowicz
Yes you are right that we want to keep PHP 7.0 because we work with Magento 2.2 which supports this version as you can see https://github.com/magento/magento2ce/blob/2.2-develop/composer.json#L11

And thank you for this PR, it is really make sense to specify PhpUnit dependency.
If you have a second please fetch latest develop and resolve conflicts, if not we'll take care of it ones we ready to merge it.

Thank you!

@lbajsarowicz
Copy link
Contributor Author

@okolesnyk Ready.

@okolesnyk okolesnyk merged commit 1dc684a into magento:develop Nov 16, 2018
@okolesnyk okolesnyk self-assigned this Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants