From 11dd0c9fce7253dfdd7b7caacf289f92358053bb Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 2 Feb 2022 10:16:06 +0100 Subject: [PATCH 1/2] Tests: add new `PreexistingPHPCSInstalledPathsConfigTest` 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. --- ...eexistingPHPCSInstalledPathsConfigTest.php | 293 ++++++++++++++++++ 1 file changed, 293 insertions(+) create mode 100644 tests/IntegrationTest/PreexistingPHPCSInstalledPathsConfigTest.php diff --git a/tests/IntegrationTest/PreexistingPHPCSInstalledPathsConfigTest.php b/tests/IntegrationTest/PreexistingPHPCSInstalledPathsConfigTest.php new file mode 100644 index 00000000..0ac25128 --- /dev/null +++ b/tests/IntegrationTest/PreexistingPHPCSInstalledPathsConfigTest.php @@ -0,0 +1,293 @@ + 'phpcs-composer-installer/preexisting-config-test', + 'require-dev' => array( + 'squizlabs/php_codesniffer' => null, + 'dealerdirect/phpcodesniffer-composer-installer' => '*', + ), + ); + + /** + * Set up test environment before each test. + */ + protected function set_up() + { + $this->createTestEnvironment(); + + /* + * Create an extra directory in the test environment, but outside of the directory used + * in the test (where Composer is run) and copy an external standard fixture to it. + */ + $this->tempExtraStndsPath = static::$tempDir . $this->tempExtraStndsSubdir; + + // Create all needed subdirectories in one go. + $path = $this->tempExtraStndsPath . '/DummySubDir'; + if (mkdir($path, 0766, true) === false || is_dir($path) === false) { + throw new RuntimeException("Failed to create the $path directory for the test"); + } + + // We only need the ruleset.xml file for the standard to be valid. + copy( + dirname(__DIR__) . '/fixtures/dummy-subdir/DummySubDir/ruleset.xml', + $this->tempExtraStndsPath . '/DummySubDir/ruleset.xml' + ); + } + + /** + * Clean up after each test. + */ + protected function tear_down() + { + $this->removeTestEnvironment(); + } + + /** + * Test correctly handling a pre-existing PHPCS configuration file which includes a pre-set, + * valid `installed_paths`. + * + * @dataProvider dataPHPCSVersions + * + * @param string $phpcsVersion PHPCS version to use in this test. + * This version is randomly selected from the PHPCS versions compatible + * with the PHP version used in the test. + * + * @return void + */ + public function testPreexistingValidInstalledPathsConfigIsKeptIntact($phpcsVersion) + { + $config = $this->composerConfig; + $config['require-dev']['squizlabs/php_codesniffer'] = $phpcsVersion; + + $this->writeComposerJsonFile($config, static::$tempGlobalPath); + + /* + * 1. Install PHPCS and the plugin. + */ + $this->assertExecute( + 'composer global install', + 0, // Expected exit code. + null, // No stdout expectation. + null, // No stderr expectation. + 'Failed to install PHPCS.' + ); + + /* + * 2. Set the installed_paths and verify it is registered correctly. + */ + $command = sprintf( + '"vendor/bin/phpcs" --config-set installed_paths %s', + escapeshellarg($this->tempExtraStndsPath) + ); + $result = $this->executeCliCommand($command, static::$tempGlobalPath); + $this->assertSame( + 0, + $result['exitcode'], + 'Exitcode for "phpcs --config-set installed_paths" did not match 0' + ); + + // Verify that the config contains the newly set value. + $result = $this->executeCliCommand('"vendor/bin/phpcs" --config-show', static::$tempGlobalPath); + $this->assertSame(0, $result['exitcode'], 'Exitcode for "phpcs --config-show" did not match 0 (first run)'); + + $expected = array( + $this->tempExtraStndsPath, + ); + + $this->assertSame( + $expected, + $this->configShowToPathsArray($result['stdout']), + 'PHPCS configuration does not show the manually set installed_paths correctly' + ); + + /* + * 3. Install an external standard. + */ + $expectedStdOut = $this->willPluginOutputShow() ? 'PHP CodeSniffer Config installed_paths set to ' : null; + $command = 'composer global require --dev phpcs-composer-installer/dummy-subdir --no-ansi -v'; + $this->assertExecute( + $command, + 0, // Expected exit code. + $expectedStdOut, // Expectation for stdout. + null, // No stderr expectation. + 'Failed to install Dummy subdir standard.' + ); + + // Verify that the originally set path is retained and the new standard is registered correctly as well. + $result = $this->executeCliCommand('"vendor/bin/phpcs" --config-show', static::$tempGlobalPath); + $this->assertSame(0, $result['exitcode'], 'Exitcode for "phpcs --config-show" did not match 0 (second run)'); + + + $expected = array( + $this->tempExtraStndsPath, + '/phpcs-composer-installer/dummy-subdir', + ); + sort($expected, \SORT_NATURAL); + + $this->assertSame( + $expected, + $this->configShowToPathsArray($result['stdout']), + 'Paths as updated by the plugin does not contain the expected paths' + ); + } + + /** + * Test correctly handling a pre-existing PHPCS configuration file which includes both + * a pre-set, valid path, as well as an invalid path in `installed_paths`. + * + * @dataProvider dataPHPCSVersions + * + * @param string $phpcsVersion PHPCS version to use in this test. + * This version is randomly selected from the PHPCS versions compatible + * with the PHP version used in the test. + * + * @return void + */ + public function testPreexistingInvalidInstalledPathsConfigIsRemoved($phpcsVersion) + { + $config = $this->composerConfig; + $config['require-dev']['squizlabs/php_codesniffer'] = $phpcsVersion; + + $this->writeComposerJsonFile($config, static::$tempLocalPath); + + /* + * 1. Install PHPCS and the plugin. + */ + $this->assertExecute( + sprintf('composer install -v --working-dir=%s', escapeshellarg(static::$tempLocalPath)), + 0, // Expected exit code. + null, // No stdout expectation. + null, // No stderr expectation. + 'Failed to install PHPCS.' + ); + + /* + * 2. Set the installed_paths and verify it is registered correctly. + */ + $command = sprintf( + '"vendor/bin/phpcs" --config-set installed_paths %s', + escapeshellarg($this->tempExtraStndsPath) + ); + $result = $this->executeCliCommand($command, static::$tempLocalPath); + $this->assertSame( + 0, + $result['exitcode'], + 'Exitcode for "phpcs --config-set installed_paths" did not match 0' + ); + + /* + * Manipulate the value of installed_paths as registered in PHPCS. + * + * Note: for the test we do this "manually". In real life, this may be a standard which + * used to be installed, but was removed without the installed_paths having been updated + * in PHPCS (prior to the plugin being used). + * + * Also note: depending on the OS and the PHP version, passing an invalid path to `--config-set` + * will error on an exception from the DirectoryIterator as used by PHPCS itself. + * The manual setting prevents this exception, but still allows us to test this use-case. + */ + $confFile = static::$tempLocalPath . '/vendor/squizlabs/php_codesniffer/CodeSniffer.conf'; + $confContents = file_get_contents($confFile); + $this->assertNotFalse($confContents); + $confContents = str_replace( + $this->tempExtraStndsSubdir, + $this->tempExtraStndsSubdir . ',path/to/somecloned-stnd', + $confContents + ); + $this->assertNotFalse(file_put_contents($confFile, $confContents)); + + // Verify that the config contains the newly set value. + $result = $this->executeCliCommand('"vendor/bin/phpcs" --config-show', static::$tempLocalPath); + $this->assertSame(0, $result['exitcode'], 'Exitcode for "phpcs --config-show" did not match 0 (first run)'); + + $expected = array( + $this->tempExtraStndsPath, + 'path/to/somecloned-stnd', + ); + sort($expected, \SORT_NATURAL); + + $this->assertSame( + $expected, + $this->configShowToPathsArray($result['stdout']), + 'PHPCS configuration does not show the manually set installed_paths correctly' + ); + + /* + * 3. Install an external standard. + */ + $expectedStdOut = $this->willPluginOutputShow() ? 'PHP CodeSniffer Config installed_paths set to ' : null; + $command = sprintf( + 'composer require --dev phpcs-composer-installer/dummy-subdir --no-ansi -v --working-dir=%s', + escapeshellarg(static::$tempLocalPath) + ); + $this->assertExecute( + $command, + 0, // Expected exit code. + $expectedStdOut, // Expectation for stdout. + null, // No stderr expectation. + 'Failed to install Dummy subdir standard.' + ); + + /* + * Verify that the valid preset path is retained, that the invalid path is removed + * and the new standard is registered correctly. + */ + $result = $this->executeCliCommand('"vendor/bin/phpcs" --config-show', static::$tempLocalPath); + $this->assertSame(0, $result['exitcode'], 'Exitcode for "phpcs --config-show" did not match 0 (second run)'); + + $expected = array( + $this->tempExtraStndsPath, + '/phpcs-composer-installer/dummy-subdir', + ); + sort($expected, \SORT_NATURAL); + + $this->assertSame( + $expected, + $this->configShowToPathsArray($result['stdout']), + 'Paths as updated by the plugin does not contain the expected paths' + ); + } + + /** + * Data provider. + * + * @return array + */ + public function dataPHPCSVersions() + { + // Test against the highest and lowest supported PHPCS version for each major + `master` + PHPCS 4.x dev. + $versions = PHPCSVersions::getHighLowEachMajor(true, true); + return PHPCSVersions::toDataprovider($versions); + } +} From c01bd6f79a78a4819ec0a44f5d38c38d829bc629 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 2 Feb 2022 15:48:12 +0100 Subject: [PATCH 2/2] 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-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+. --- src/Plugin.php | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/Plugin.php b/src/Plugin.php index 75653e49..1cb5af0f 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -234,9 +234,9 @@ private function loadInstalledPaths() { if ($this->isPHPCodeSnifferInstalled() === true) { $this->processExecutor->execute( - 'phpcs --config-show', + $this->getPhpcsCommand() . ' --config-show', $output, - $this->composer->getConfig()->get('bin-dir') + $this->getPHPCodeSnifferInstallPath() ); $regex = sprintf(self::PHPCS_CONFIG_REGEX, self::PHPCS_CONFIG_KEY); @@ -287,27 +287,16 @@ private function saveInstalledPaths() self::PHPCS_CONFIG_KEY ); - // Determine the path to the main PHPCS file. - $phpcsPath = $this->getPHPCodeSnifferInstallPath(); - if (file_exists($phpcsPath . '/bin/phpcs') === true) { - // PHPCS 3.x. - $phpcsExecutable = './bin/phpcs'; - } else { - // PHPCS 2.x. - $phpcsExecutable = './scripts/phpcs'; - } - // Okay, lets rock! $command = vsprintf( - '%s %s %s', + '%s %s', array( - 'php executable' => $this->getPhpExecCommand(), - 'phpcs executable' => $phpcsExecutable, - 'arguments' => implode(' ', $arguments), + 'phpcs command' => $this->getPhpcsCommand(), + 'arguments' => implode(' ', $arguments), ) ); - $exitCode = $this->processExecutor->execute($command, $configResult, $phpcsPath); + $exitCode = $this->processExecutor->execute($command, $configResult, $this->getPHPCodeSnifferInstallPath()); if ($exitCode === 0) { $exitCode = $this->verifySaveSuccess(); } @@ -359,6 +348,30 @@ private function verifySaveSuccess() return $exitCode; } + /** + * Get the command to call PHPCS. + */ + protected function getPhpcsCommand() + { + // Determine the path to the main PHPCS file. + $phpcsPath = $this->getPHPCodeSnifferInstallPath(); + if (file_exists($phpcsPath . '/bin/phpcs') === true) { + // PHPCS 3.x. + $phpcsExecutable = './bin/phpcs'; + } else { + // PHPCS 2.x. + $phpcsExecutable = './scripts/phpcs'; + } + + return vsprintf( + '%s %s', + array( + 'php executable' => $this->getPhpExecCommand(), + 'phpcs executable' => $phpcsExecutable, + ) + ); + } + /** * Get the path to the current PHP version being used. *