AbstractSniffUnitTest: bugfix - don't save config between tests #2899
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
The `setCliValues()` method allows for changing the PHPCS configuration for specific test case files, but the `AbstractSniffUnitTest` does not offer a mechanism to undo those changes for subsequent tests as the instance of the `Config` class is saved and reused. In effect, that means that any change in the `config` values is carried through to all subsequent tests unless a subsequent test specifically overrules the change again. While setting up the config anew for each test will make the tests slightly slower, it will prevent this "bleed through" of config values between tests. I've left the check for the global variable in place in case external standards would set it themselves intentionally. At least this way, this change will not break those tests. Alternatively a `tearDown()`-like method could be offered from the `AbstractSniffUnitTest` class to allow for resetting any config values set by tests.
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.
The
setCliValues()method allows for changing the PHPCS configuration for specific test case files, but theAbstractSniffUnitTestdoes not offer a mechanism to undo those changes for subsequent tests as the instance of theConfigclass is saved and reused.In effect, that means that any change in the
configvalues is carried through to all subsequent tests unless a subsequent test specifically overrules the change again.While setting up the config anew for each test will make the tests slightly slower, it will prevent this "bleed through" of config values between tests.
I've left the check for the global variable in place in case external standards would set it themselves intentionally. At least this way, this change will not break those tests.
Alternatively a
tearDown()-like method could be offered from theAbstractSniffUnitTestclass to allow for resetting any config values set by tests.