TokenNameTest: ensure all polyfilled tokens are tested #942
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
For the record: this race condition is most likely to happen when running the tests using PHPUnit Phar files with the |
Other tooling packages will often also polyfill PHP native tokens and are likely to use different values for the token constant (generally an integer above 10.000). A number of these tool have a habit to autoload the file containing the polyfilled token constants via the Composer `autoload - files` directives. When this happens, this can interfere with the functioning of PHPCS. While it will be rare for this to become problematic for normal PHPCS runs (as translation between token constants and their names and visa versa is rarely done in runtime-code), it is definitely something which can cause problems while running the tests for PHPCS itself. Those type of issues will be hard to debug in the tests as this is a sort of race-condition. So to make it more obvious what is going on if this specific race-condition is happening, I'm going to make it a requirement for all PHP native polyfilled tokens to be tested via the `TokenNameTest` class. This will ensure that if the race-condition is happening, that test will fail, providing a valuable clue for solving the other resulting test failure(s).
|
Rebased without changes in hopes of finally getting Coveralls to report. |
jrfnl
force-pushed
the
feature/tokennametest-always-test-php-native-polyfills
branch
from
47edd7a to
cf0c547
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Other tooling packages will often also polyfill PHP native tokens and are likely to use different values for the token constant (generally an integer above 10.000).
A number of these tools have a habit to autoload the file containing the polyfilled token constants via the Composer
autoload - filesdirectives. When this happens, this can interfere with the functioning of PHPCS.While it will be rare for this to become problematic for normal PHPCS runs (as translation between token constants and their names and visa versa is rarely done in runtime-code), it is definitely something which can cause problems while running the tests for PHPCS itself.
Those type of issues will be hard to debug in the tests as this is a sort of race-condition. So to make it more obvious what is going on if this specific race-condition is happening, I'm going to make it a requirement for all PHP native polyfilled tokens to be tested via the
TokenNameTestclass.This will ensure that if the race-condition is happening, that test will fail, providing a valuable clue for solving the other resulting test failure(s).
Suggested changelog entry
N/A