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

Tests: add new RootPackageHandlingTest + bugfix #175

Merged
merged 3 commits into from May 28, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Apr 18, 2022

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 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.

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 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:

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.:

  • 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

Related Issues

Related to #92

tests/TestCase.php Outdated Show resolved Hide resolved
Potherca
Potherca previously approved these changes May 25, 2022
@jrfnl jrfnl force-pushed the feature/tests-new-rootpackage-handling-test-and-bugfix branch from 8fc19a1 to 93bee80 Compare May 25, 2022 20:22
@Potherca Potherca self-requested a review May 26, 2022 15:14
Potherca
Potherca previously approved these changes May 26, 2022
jrfnl added 3 commits May 27, 2022 16:26
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 jrfnl force-pushed the feature/tests-new-rootpackage-handling-test-and-bugfix branch from 93bee80 to 2de5e4b Compare May 27, 2022 14:26
@jrfnl
Copy link
Member Author

jrfnl commented May 27, 2022

Rebased without changes after the merge of #180 to get a clean and passing build. Will merge after the build passes.

@jrfnl jrfnl merged commit 8504433 into master May 28, 2022
@jrfnl jrfnl deleted the feature/tests-new-rootpackage-handling-test-and-bugfix branch May 28, 2022 06:24
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.

Invalid "installed_paths" config value
2 participants