-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Tests: add new RootPackageHandlingTest + bugfix #175
Merged
jrfnl
merged 3 commits into
master
from
feature/tests-new-rootpackage-handling-test-and-bugfix
May 28, 2022
Merged
Tests: add new RootPackageHandlingTest + bugfix #175
jrfnl
merged 3 commits into
master
from
feature/tests-new-rootpackage-handling-test-and-bugfix
May 28, 2022
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
Potherca
reviewed
May 25, 2022
Potherca
previously approved these changes
May 25, 2022
jrfnl
force-pushed
the
feature/tests-new-rootpackage-handling-test-and-bugfix
branch
from
May 25, 2022 20:22
8fc19a1
to
93bee80
Compare
Potherca
previously approved these changes
May 26, 2022
New helper method to recursively copy a (fixture) directory with all its files to another location, typically, one of the system temp test directories. If the `$srcDir` contains a `composer.json` file, it will get the same `TestCase::writeComposerJsonFile()` special treatment, meaning that the artifacts directory will be injected, as well as plugin permissions and more.
This new test class tests that the plugin correctly registers standards found in the root package if it is an external standard. Notes: * This test is about Composer and the plugin, so does not need to be tested against multiple PHPCS versions. This test class covers the following bugs previously reported: * Dealerdirect/phpcodesniffer-composer-installer issue 19 * Dealerdirect/phpcodesniffer-composer-installer PR 21 * Dealerdirect/phpcodesniffer-composer-installer issue 20 * Dealerdirect/phpcodesniffer-composer-installer PR 25 * Dealerdirect/phpcodesniffer-composer-installer issue 32
…dard` type (bug fix) In the "olden days" of PHPCS 1.x, it was customary for projects to call their custom project ruleset `ruleset.xml` instead of `[.]phpcs.xml[.dist]`. This could lead to a root package with such an _old-style_ PHPCS ruleset being registered as if it were a proper PHPCS external standard. This was previously reported in issue 32. The fix I'm proposing now, applies the same rules as for `vendor` installed standards to the root package, i.e.: * Must have the `phpcodesniffer-standard` type set in the `composer.json` file AND * Must have a `ruleset.xml` file. Root packages which do not comply with _both_ these rules will no longer be considered for registration with PHPCS. This should also make the plugin slightly faster for those packages which do not have external standards, but do have the plugin installed. Includes removing some dead code (condition which could never be `true`) which was loosely related to this. Fixes 32
jrfnl
force-pushed
the
feature/tests-new-rootpackage-handling-test-and-bugfix
branch
from
May 27, 2022 14:26
93bee80
to
2de5e4b
Compare
Rebased without changes after the merge of #180 to get a clean and passing build. Will merge after the build passes. |
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.
Proposed Changes
TestCase: new recursiveDirectoryCopy() method
New helper method to recursively copy a (fixture) directory with all its files to another location, typically, one of the system temp test directories.
If the
$srcDir
contains acomposer.json
file, it will get the sameTestCase::writeComposerJsonFile()
special treatment, meaning that the artifacts directory will be injected, as well as plugin permissions and more.Tests: add new RootPackageHandlingTest
This new test class tests that the plugin correctly registers standards found in the root package if it is an external standard.
Notes:
This test class covers the following bugs previously reported:
Plugin: only search root package when it has the
phpcodesniffer-standard
type (bug fix)In the "olden days" of PHPCS 1.x, it was customary for projects to call their custom project ruleset
ruleset.xml
instead of[.]phpcs.xml[.dist]
.This could lead to a root package with such an old-style PHPCS ruleset being registered as if it were a proper PHPCS external standard.
This was previously reported in issue #32.
The fix I'm proposing now, applies the same rules as for
vendor
installed standards to the root package, i.e.:phpcodesniffer-standard
type set in thecomposer.json
file ANDruleset.xml
file.Root packages which do not comply with both these rules will no longer be considered for registration with PHPCS. This should also make the plugin slightly faster for those packages which do not have external standards, but do have the plugin installed.
Includes removing some dead code (condition which could never be
true
) which was loosely related to this.Fixes #32
Related Issues
Related to #92