Tests: add new PreexistingPHPCSInstalledPathsConfigTest + bug fix
#167
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
This new test class tests that the plugin handles a pre-existing PHPCS configuration file, which includes an `installed_paths` setting, correctly.
These tests verify:
- That the plugin does not remove or alter any valid paths which already existed in `installed_paths`.
- That the plugin removes invalid paths which already existed in `installed_paths`.
Notes:
* While PHPCS behaves the same whether installed locally or globally, it is still necessary to run the tests in both type of environments.
Bug composer/composer 10504 (discovered while writing these tests) is example of the reason why.
To not create too much redundancy, I've elected to run one of the tests via Composer global and the other one via local.
* As the output of the `config-show` command in PHPCS has changed across PHPCS version, it is necessary to test against multiple PHPCS versions.
PR 80 in response to bug 79 fixed the issue of the command to set the installed paths failing when the system default PHP version was being used instead of the PHP version used by Composer.
However, PHPCS is also called for the `--config-show` command to retrieve the originally set paths as well as to verify a successful save.
This `--config-show` command was not adjusted at the time and was still being run using the system default PHP version, which could lead to valid `installed_paths` which were present before the installation of the plugin being removed, as `--config-show` would silently fail, leading to the initial `$this->installedPaths` being empty.
This commit fixes that by:
1. Introducing a new `getPhpcsCommand()` method which will cobble together the PHP executable + the PHPCS executable to a runnable command.
2. Using that new method in both places in the plugin were PHPCS is called.
This fixes the bug and improves consistency in the plugin.
It also happens to work as a work-around for upstream by composer/composer 10504 as it removes the call to `$this->composer->getConfig()->get('bin-dir')` which would result in an incorrect execution directory on Windows with Composer 2.2.2+.
jrfnl
force-pushed
the
feature/tests-new-preexistingphpcsinstalledpathsconfig-test
branch
from
9109ff4
to
c01bd6f
Compare
Potherca
approved these changes
Mar 5, 2022
Potherca
deleted the
feature/tests-new-preexistingphpcsinstalledpathsconfig-test
branch
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
Tests: add new
PreexistingPHPCSInstalledPathsConfigTestThis new test class tests that the plugin handles a pre-existing PHPCS configuration file, which includes an
installed_pathssetting, correctly.These tests verify:
installed_paths.installed_paths.Notes:
Bug [2.2.2+] $this->composer->getConfig()->get('bin-dir') returns incorrect path on Windows composer/composer#10504 (discovered while writing these tests) is example of the reason why.
To not create too much redundancy, I've elected to run one of the tests via Composer global and the other one via local.
config-showcommand in PHPCS has changed across PHPCS version, it is necessary to test against multiple PHPCS versions.Plugin: add new
getPhpcsCommand()method (bug fix)PR #80 in response to bug #79 fixed the issue of the command to set the installed paths failing when the system default PHP version was being used instead of the PHP version used by Composer.
However, PHPCS is also called for the
--config-showcommand to retrieve the originally set paths as well as to verify a successful save.This
--config-showcommand was not adjusted at the time and was still being run using the system default PHP version, which could lead to validinstalled_pathswhich were present before the installation of the plugin being removed, as--config-showwould silently fail, leading to the initial$this->installedPathsbeing empty.This commit fixes that by:
getPhpcsCommand()method which will cobble together the PHP executable + the PHPCS executable to a runnable command.This fixes the bug and improves consistency in the plugin.
It also happens to work as a work-around for upstream by composer/composer#10504 as it removes the call to
$this->composer->getConfig()->get('bin-dir')which would result in an incorrect execution directory on Windows with Composer 2.2.2 - 2.2.5 (a fix for the issues was included in Composer itself in release 2.2.6).Related to #92