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

Replace a lot of near duplicate unit test functions with data providers. #149

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Aug 8, 2016

Makes maintenance of the unit tests easier if something would change and makes adding new unit tests a breeze.

This change also makes the unit tests more complete as often only the 'error' case was tested and not the 'noViolation' case. This has now been rectified.

The number of tests being run will now also be reported more accurately.

For this PR I have focused on the test files where the most advantage could be gained from using data providers.

Additional changes:

  • In most files I touched: Added a class constant for the test file name and used that throughout the class.
  • Add proper skip test indication for tests being skipped based on PHPCS version.
  • Renamed the NewAnonymousClasses test file (contained a typo).
  • Show deprecation for removed ereg extension and show pcre as an alternative. (DeprecatedExtensions)
  • Show removal for ereg and mysql functions. (DeprecatedFunctions).
  • Show deprecation for mysqli functions. (DeprecatedFunctions).
  • Corrected some version numbers. (DeprecatedFunctions).
  • Removed a few setUp() functions which did nothing but fall through to the parent.
  • A number of noViolation unit test cases for the NonStaticMagicMethods sniff existed, but most were not actually being tested. That's now corrected.
  • Some documentation improvements.

This is a first iteration for refactoring of the unit tests.
A further abstraction of a lot of these unit tests is definitely possible in the future.

Makes maintenance of the unit tests easier if something would change and makes adding new unit tests a breeze.

This change also makes the unit tests more complete as often only the 'error' case was tested and not the 'noViolation' case. This has now been rectified.

The number of tests being run will be now also be reported more accurately.

For this PR I have focused on the test files where the most advantage could be gained from using data providers.

Additional changes:
* Added a constant for the test file name and used that throughout the tests in the files I touched.
* Add proper skip test indication for tests being skipped based on PHPCS version.
* Renamed the `NewAnonymousClasses` test file (contained a typo).
* Show deprecation for removed `ereg` extension and show `pcre` as an alternative. (`DeprecatedExtensions`)
* Show removal for `ereg` and `mysql` functions. (`DeprecatedFunctions`).
* Show deprecation for `mysqli` functions. (`DeprecatedFunctions`).
* Corrected some version numbers. (`DeprecatedFunctions`).
* Removed a few `setUp()` functions which did nothing but fall through to the parent.
* A number of `noViolation` unit test cases for the `NonStaticMagicMethods` sniff existed, but most were not actually being tested. That's now corrected.
* Some documentation improvements.

This is a first iteration for refactoring of the unit tests.
A further abstraction of a lot of these unit tests is definitely possible in the future.
@wimg wimg merged commit 3fd823f into PHPCompatibility:master Aug 11, 2016
@wimg
Copy link
Member

wimg commented Aug 11, 2016

Absolutely amazing !

@jrfnl
Copy link
Member Author

jrfnl commented Aug 11, 2016

Glad you like what I'm doing.
Some more PRs coming up, was waiting for this one as they would otherwise all clash on the unit tests.

@jrfnl jrfnl deleted the feature/refactor-unit-tests-to-data-providers branch August 11, 2016 01:13
@jrfnl
Copy link
Member Author

jrfnl commented Aug 11, 2016

PS.: what are you still doing up at this hour ? 😎

@wimg
Copy link
Member

wimg commented Aug 11, 2016

Look who's talking. You've been sending PRs in the middle of the night for the last week or so ;-)

@jrfnl jrfnl added this to the 7.0.3 milestone Apr 30, 2017
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.

None yet

2 participants