Files/TestDoubles: support modern PHP, check paths case-sensitively and other improvements #344
Conversation
This file contains 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 `public` `$doubles_path` property originally expected a string value. This was changed to an array in YoastCS 1.1.0, though string values were still handled via this BC-layer. Enough time has passed by now, so this BC-layer should now be removed.
This is already checked via the `Generic.Files.OneObjectStructurePerFile` sniff, which is included in the WordPress Coding Standards. No need for doubling this check here. Includes removing the associated unit tests. Includes removing the code sample related to mocks/doubles being in the same file as the test class from the XML docs.
... for a few edge cases previously not covered by tests.
As the sniff class is now `final` (since PR 319), there is no need for any `protected` properties, so let's make this `private`.
Lower nesting levels and remove `elseif` when not needed. :point_right: This commit will be easiest to review while ignoring whitespace changes.
Move variable definitions to the point in the code flow where they will be used. No need to define them earlier.
Ensure all test files end on a new line.
…Helper classes and check case-sensitively Aside from starting to use the new `PathHelper` and `PathValidationHelper` utility function classes, this commit also changes the path comparison from case-INsensitive to case-sensitive, which, what with the change to strict PSR-4 compliance for all test files, including double/mock files, is a change which needs to be made anyway. The tests sub-directory names have been updated to proper case to be in line with this change, the inline property settings in test case files have been updated too. On top of that, a couple of dedicated new test cases have been added to verify the case-sensitive handling of the `double_path` property. Includes updating two pre-existing tests to pass duplicate excluded files in different ways.
The `public $doubles_path` contained an arbitrary default value. While this default value is applicable for some of the Yoast codebases, it still needs to be duplicated in the ruleset due to a bug/missing feature in PHPCS in how array properties containing default values are handled. Having this arbitrary default value in the sniff also codifies some Yoast-specific repo layout logic in the sniff, while sniffs, generally speaking should be code-base agnostic. With that in mind, I'm removing the default value. The `Yoast` ruleset contains this value anyhow (due to the bug/missing feature) and that is the more appropriate place for code-base specific/organisation specific property values. Includes updating the test case files to allow for this change. Refs: * squizlabs/PHP_CodeSniffer 2154 * squizlabs/PHP_CodeSniffer 2228
... to be in line with the change in the test directory structure in the plugin repos and to be compliant with the change to (case-sensitive) PSR-4 for all test files.
Includes tests covering this change.
... to more closely match what the sniff is looking for.
... to remove the presumption that all test fixtures are classes.
jrfnl
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Files/TestDoubles: remove $doubles_path BC-layer
The
public$doubles_pathproperty originally expected a string value. This was changed to an array in YoastCS 1.1.0, though string values were still handled via this BC-layer.Enough time has passed by now, so this BC-layer should now be removed.
Files/TestDoubles: remove "one object per file" check
This is already checked via the
Generic.Files.OneObjectStructurePerFilesniff, which is included in the WordPress Coding Standards. No need for doubling this check here.Includes removing the associated unit tests.
Includes removing the code sample related to mocks/doubles being in the same file as the test class from the XML docs.
Files/TestDoubles: add extra tests
... for a few edge cases previously not covered by tests.
Files/TestDoubles: make property private
As the sniff class is now
final(since PR #319), there is no need for anyprotectedproperties, so let's make thisprivate.Files/TestDoubles: minor documentation tweaks
Files/TestDoubles: minor code tweaks [1]
Lower nesting levels and remove
elseifwhen not needed.👉 This commit will be easiest to review while ignoring whitespace changes.
Files/TestDoubles: minor code tweaks [2]
Move variable definitions to the point in the code flow where they will be used. No need to define them earlier.
Files/TestDoubles: minor test tweaks
Ensure all test files end on a new line.
Files/TestDoubles: use PHPCSUtils in more places
Files/TestDoubles: implement use of new PathHelper and PathValidationHelper classes and check case-sensitively
Aside from starting to use the new
PathHelperandPathValidationHelperutility function classes, this commit also changes the path comparison from case-INsensitive to case-sensitive, which, what with the change to strict PSR-4 compliance for all test files, including double/mock files, is a change which needs to be made anyway.The tests sub-directory names have been updated to proper case to be in line with this change, the inline property settings in test case files have been updated too.
On top of that, a couple of dedicated new test cases have been added to verify the case-sensitive handling of the
double_pathproperty.Includes updating two pre-existing tests to pass duplicate excluded files in different ways.
Files/TestDoubles: make the sniff codebase agnostic
The
public $doubles_pathcontained an arbitrary default value.While this default value is applicable for some of the Yoast codebases, it still needs to be duplicated in the ruleset due to a bug/missing feature in PHPCS in how array properties containing default values are handled.
Having this arbitrary default value in the sniff also codifies some Yoast-specific repo layout logic in the sniff, while sniffs, generally speaking should be code-base agnostic.
With that in mind, I'm removing the default value. The
Yoastruleset contains this value anyhow (due to the bug/missing feature) and that is the more appropriate place for code-base specific/organisation specific property values.Includes updating the test case files to allow for this change.
Refs:
YoastCS/TestDoubles sniff: update the doubles_path property value
... to be in line with the change in the test directory structure in the plugin repos and to be compliant with the change to (case-sensitive) PSR-4 for all test files.
Files/TestDoubles: allow for doubling PHP 8.1+ enums
Includes tests covering this change.
Files/TestDoubles: minor improvements to the XML docs
... to more closely match what the sniff is looking for.
Files/TestDoubles: minor tweaks to the error messages
... to remove the presumption that all test fixtures are classes.