-
Notifications
You must be signed in to change notification settings - Fork 964
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] tests code cleanup #618
Conversation
Reason - when the class is renamed or removed, it will be detected much faster. It helps with the static analysis of the code (also used by IDEs)
assertEquals() means == assertSame() is === Can cause tests not detecting issues, because different instances of same class with same data are equal, but not the same.
06debd9
to
5875d52
Compare
@polyfractal I've rebased this and it is ready for review. |
LGTM, thanks for these :) Looking through the affected code, it reminds me that there are still a few unit tests that still rely on an externally running cluster (e.g. I'll open an issue so that these don't continue being forgotten about :) |
* [TEST] use ::class instead of classname string Reason - when the class is renamed or removed, it will be detected much faster. It helps with the static analysis of the code (also used by IDEs) * [TEST] use assertSame() instead of assertEquals() assertEquals() means == assertSame() is === Can cause tests not detecting issues, because different instances of same class with same data are equal, but not the same. * [TEST] convert array() to [] syntax * [TEST] $this->assert* should be used See sebastianbergmann/phpunit#1914 (comment)
* [TEST] use ::class instead of classname string Reason - when the class is renamed or removed, it will be detected much faster. It helps with the static analysis of the code (also used by IDEs) * [TEST] use assertSame() instead of assertEquals() assertEquals() means == assertSame() is === Can cause tests not detecting issues, because different instances of same class with same data are equal, but not the same. * [TEST] convert array() to [] syntax * [TEST] $this->assert* should be used See sebastianbergmann/phpunit#1914 (comment)
Depends on #612 (should be merged first)merge 612Reasoning for each change is included in the individual commit messages.