-
Notifications
You must be signed in to change notification settings - Fork 132
MQE-1750: unit tests #476
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
MQE-1750: unit tests #476
Conversation
1 similar comment
* | ||
* @var ComposerInstall | ||
*/ | ||
private static $composer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change this to just be private $composer;
and change setUpBeforeClass()
to just setUp()
.
Since this is initialized only once in the test, the object's state is subject to change due to test's manipulation, which set a bad pattern if we ever do any state tracking in that class in the future.
Also being the class under test, we should really initialize once per test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
* | ||
* @var ComposerPackage | ||
*/ | ||
private static $composer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests look good to me!
Diffed before/after of magento2ce enabledDirectoryPaths
and no difference detected, nice work 👍
Description
Fixed Issues (if relevant)
Contribution checklist