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

Add initial integration test setup and first few tests #153

Merged
merged 14 commits into from Mar 5, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Feb 1, 2022

Introduction

This PR creates an initial test framework to allow for creating and running integration/end-to-end tests for this plugin.

The framework as I have set it up:

  • Allows for running tests both as a "project local" as well as a Composer global install without screwing up the Composer global environment of the user running the tests.
    Tests are run in the system temp directory and the tests clean up after themselves.
  • Will ensure that the tests will run with the plugin as it is in the branch being tested.
  • As the tests start new PHP processes, the framework ensures that the PHP version used to initiate the test run will be used in those processes. This prevents a test run on a dev machine using the system default PHP version, which would skew the test results.
  • Similarly, the framework can be told to use a specific Composer phar file to run the tests with, rather than the system default version.
  • The framework also allows for setting up "fake" PHPCS standards for use in the tests, which will make writing the tests simpler as we don't need to keep track of changes in external standards and don't need to take those into account in the tests.
  • Tests can be run against multiple PHPCS versions using data providers and a helper class is included to get a specific or random) selection of versions to use compatible with the PHP version against which the tests are being run.
  • And to aid in debugging failing tests, it includes a TestListener which will spit out all the CLI output seen for a particular test when the test fails.

The tests included in this PR are largely the tests which were already previously being run via the GH Actions workflow, though they are now encapsuled within a PHPUnit based framework and will be run via GH Actions against all supported PHP version, multiple versions of Composer and on multiple OSes.

A lot more tests are still needed, but those should be relatively straight-forward to add once this initial framework is in place and I have already started preparing some follow-up PRs to be pulled once this PR has been merged.

⚠️ This PR depends on PRs #147 and #152 both being accepted and merged.

👉🏻 As this is a huge PR, I have split it up into atomic commits telling the "story" and I would strongly recommend reviewing this PR by looking at each commit separately.

Proposed Changes

Tests: create initial setup

  • Require the PHPUnit Polyfills as a basis for the tests.
    The PHPUnit Polyfills will ensure a compatible PHPUnit version will be available and will allow for writing tests for the latest version of PHPUnit (9.x), while still being able to run the tests on PHPUnit 4-8.
  • Include a basic test bootstrap file which will allow for running the tests both via a Composer installed version of PHPUnit as well as via a PHPUnit PHAR file.
    Note: the bootstrap file will try to determine where the composer.phar file is located. For running the tests locally and/or overruling the default version of Composer, a COMPOSER_PHAR environment variable can be set in a local phpunit.xml overload file.
  • Include information about the test setup in the CONTRIBUTING documentation.
  • Includes adding new jobs to the "Quick test" and "Integration test" GH Actions workflows.
    Once the existing tests in the old workflows have been converted to the new test setup, the old workflows can be removed.
  • Minor tweaks to various repository config files.

Note: this test setup presumes that the proposal to drop support for PHP 5.3 - see issue #145 - will be accepted as the PHPUnit Polyfills require PHP >= 5.4.

Refs:

TestCase: add helper methods for setup/teardown

As most tests will need to run a composer install, tests should be run in separate, temporary directories.

With that in mind, I'm adding two helper methods to the base TestCase class:

  • createTestEnvironment() - this method will create a PHPCSPluginTest/TestClassName_uniqueID/local and a PHPCSPluginTest/TestClassName_uniqueID/global directory in the system temp directory and set the Composer global HOME directory to the created global directory.
  • removeTestEnvironment() - this method will remove the created directories and reset the Composer global HOME directory to its original location.

Notes:

  • The paths to the created directories will be added to the $tempDir, $tempGlobalPath and $tempLocalPath properties of the class and can be used in the tests.
  • The methods (and associated properties) have been made static on purpose to allow for these methods to be used both in setUp()/tearDown() fixtures as well as in setUpBeforeClass()/tearDownAfterClass() fixtures.
    The latter would be useful for a test class needing only one environment with various test methods depending on each other and building on each other.
    This is also the reason that these methods are set up as helpers instead of fixtures, as this way, the decision what fixtures are most appropriate can be made for each individual concrete test class.
  • A hard cleanup of the PHPCSPluginTest directory in the system temp directory has been added to the test bootstrap to make sure that any stray "old" test directories will be removed before a new test run.

Includes a static TestCase::onWindows() helper method to determine whether or not the tests are being run on Windows.

TestCase: add helper method for on-the-fly composer.json creation

This adds a test helper method, which, given an array with a Composer configuration and a target location, will on-the-fly create a composer.json file for use in a test.

The method will automatically add two additional settings to the composer.json file:

  • It will add a install-codestandards script to run the plugin on demand.
  • It will add the allow-plugins permission, which is needed for Composer 2.2.

By adding these keys automatically, this kind of "noise" is not needed in the actual test classes.

The method is static so as to allow for it to be used in setUpBeforeClass()/tearDownAfterClass() fixtures.

Tests: create zip package artifact for the current plugin code

In normal circumstances, a composer install will download a version of the plugin from Packagist.
This will not work for the tests however, as the development version of the plugin being tested will generally not be available on Packagist.

With this in mind, we need a work-around to ensure that the tests are run with the current version of the plugin as per the last commit in the current branch.

I've implemented this by using the repositories - artifact feature of Composer.

This works as follow:

  • In the test bootstrap, ahead of the test run, the plugin is zipped up as a package and placed in the tests/artifact directory.
  • Whenever a composer install/update command is run within the test suite, the repositories-artifact key is injected into the composer.json when the composer.json file is written to the temp directory used for the test.
  • With this in place, Composer will install the plugin from the zipped file in the tests/artifact directory instead of downloading from Packagist.

Notes:

  • The version number to be used for the zip artifact of the plugin is defined in the test bootstrap file.
    This number may need updating once in a while to ensure it is higher than the latest released version of the plugin.
  • In the CreateComposerZipArtifacts class, three properties are used to determine which files to exclude from the package - $excludedFiles, $excludedExtensions and $excludedDirs.
    Depending on new files/directories being introduced in the package, these lists may need to be updated once in a while.

Ref: https://getcomposer.org/doc/05-repositories.md#artifact

TestCase: add helper methods to run CLI commands and custom assertions

This commit adds six additional methods to the base TestCase class:

  • executeCliCommand(): a helper method to execute a CLI command and return the exit code, the contents of stdout and the content of stderr for use in assertions.
  • stabilizeCommand(): a helper method which is automatically called from the executeCliCommand() method. This method will:
    • Ensure that the PHP version which was used to initiate the test run will also be used in the CLI command.
      Without this, the system default PHP version would be used for CLI commands, which may be a different version from the PHP version used to run the tests and would skew the test results.
    • Ensure that the Composer version as set up using the env variable is used in the tests.
      Again, this will prevent the system default version of Composer being used, which may skew the results when the tests are intended to be run against a specific version of Composer.
    • Adds the --no-interaction flag to all Composer CLI commands to prevent tests hanging when interaction would be expected.
    • Adds quotes around a command when the tests are run on Windows in combination with PHP < 8 to ensure cross-platform compatibility of the CLI commands.
  • getPhpcsCommand(): a helper method to get the path to the vendor installed PHPCS entry point file.
  • maybeStripColors(): a helper method to compare CLI output. This method is still WIP and will probably still need some tweaking.
    This method will take an output expectation and an actual output and will attempt to strip CLI colour codes from the actual output when the output expectation does not contain colour codes. This way, expected colour codes can still be tested by including them in the output expectation.
    Note: until the bugs in this method have been smoothed out, it is recommended to use --no-ansi CLI arguments in CLI commands not testing for colour codes.
  • assertExecute(): a custom assertion which will run a given CLI command, optionally in a specific working directory and will subsequently assert that the exit code is the same as expected and the stdout and stderr outputs contain the expected output.
    For more complex assertions against the output of CLI commands, the executeCliCommand() can be used directly in tests.
    The StringContainsString assertions used for stdout and stderr will attempt to strip color codes from the output before doing a compare using the maybeStripColors() method.
  • assertComposerValidates(): a custom assertion to verify that a given composer.json file validates for use in the tests.

A number of these methods have been made static to allow for them to be used in conjunction with setUpBeforeClass() fixtures.

Tests: add TestListener to log and display CLI output for failing tests

While not necessarily the most pretty solution (output is displayed in the middle of the progress report), this allows for access to the composer.json content, the CLI commands and their output for any test which failed, which will be helpful when debugging test failures, especially in CI.

Tests: add initial baseline test

These tests verify:

  • That the plugin can be installed and functions correctly with the full range of supported PHPCS versions.
    While the test is not run against the full range of supported PHPCS version each time, due to the random PHPCS version selection, all supported PHPCS versions will be tested over time in CI.
  • That the plugin runs when installed.
  • That the PHPCS native standards are the only recognized standards when no external standards are available.
  • That no `CodeSniffer.conf``file gets created when no external standards are found.

These tests will be run in both a Composer global as well as a Composer project local environment.

The tests use a dataprovider which will randomly select two PHPCS versions compatible with the PHP version on which the test is being run + PHPCS dev-master + PHPCS 4.x (dev version for the next major).

As which PHPCS versions are compatible with which PHP versions needs quite some logic, a separate helper class PHPCSVersions is introduced, which encapsulates that knowledge.

The PHPCSVersions class contains a number of (static) methods as helpers. These can be used in any test class:

  • get(): retrieves an array with a specific number of PHPCS versions valid for the current PHP version.
  • getHighLow(): retrieves an array of the highest and lowest PHPCS versions valid for the current PHP version.
  • getHighLowEachMajor(): retrieves an array of the highest and lowest supported PHPCS versions for each PHPCS major (and valid for the current PHP version).
  • getRandom(): gets a random PHPCS version which is valid for the current PHP version.
  • toDataprovider(): converts a version array to an array suitable for use as a PHPUnit dataprovider.
  • getSupportedVersions(): retrieves an array with all PHPCS versions valid for the current PHP version.
  • isNextMajorSupported(): determines whether the current PHP version is supported on the "next major" branch of PHPCS.
  • getStandards(): retrieves an array of the names of the PHPCS native standards for a particular PHPCS version.

Generally speaking, the dev-master and 4.x branch will not be included in the retrieved ranges, but can be specifically requested by passing optional parameters.

This PHPCSVersions class will need semi-regular updates when new versions of PHPCS are released and new PHP versions are released.

Tests: move test with external standard from CI to test class

This moves the test which was being run via GH Actions to a test class in the integration test suite.

Differences between the previous and new setup:

  • The previous test via GH Actions would only fail on the exit code not being 0 for any of the steps.
    The new PHPUnit based tests executes more detailed assertions on the results of the commands being run in the tests.
  • The previous test would execute the test run of PHPCS against the src/Plugin.php file in this package.
    The new PHPUnit based tests will execute against a simple PHP file created on the fly.
  • The previous test via GH Actions would only run for a Composer local situation.
    The new PHPUnit based tests cover both a Composer global as well as a local setup.
  • The matrix against which the tests are run is different.
    In the old situation, the test would only be run on a fixed set of PHP/PHPCS combinations with Composer 2.x on Ubuntu.
    In the new situation, the tests will run against both Composer 1.x, as well as 2.x on both Ubuntu and Windows.
    Same as before, it will be run against all supported PHP versions (in the extended integrationtest workflow), but the PHPCS versions to test against are more varied and - in part - randomly selected out of the PHPCS version compatible with the current PHP version, which results in more comprehensive testing of these scenarios.

Includes:

  • Adding a new createFile() helper method to the base TestCase to generate a (PHP) file to do a test run against.
  • Removing the "old" integration test from the GH Actions script.

Tests: set up a fake PHPCS standard as a fixture and create an artifact of it

The tests in the RegisterExternalStandardsTest class contain quite some work-arounds to handle changes made over time to the external PHPCompatibility standard.

The PHPCompatibility standard was chosen for use in the tests as it is one of the rare standards which still supports both PHPCS 2.x as well as 3.x.

All the same, to improve the stability of the tests and to simplify the actual test code, using "fake" PHPCS standards set up as fixtures as part of this test suite will make life easier.

This commit creates the initial setup for that, by:

  • Adding code to the CreateComposerZipArtifacts class to handle creating Composer package zip artifacts for each subdirectory of the tests/fixtures directory.
  • Calling that code from the test bootstrap file to ensure all test artifacts are zipped up and ready for use ahead of running the tests.
  • Adding an initial "fake" DummySubDir standard as a test fixture.
  • Adding a README file to the tests/fixtures directory with the rationale and a list of available fixtures.

RegisterExternalStandardsTest: switch out PHPCompatibility for the test fixture

This replaces the use of the PHPCompatibility standard in the RegisterExternalStandardsTest tests with the use of the dummy-subdir fake standard fixture.

RegisterExternalStandardsTest: simplify check whether PHPCS can run with the standard

To check whether PHPCS can run with the standard, previously, a scan on a simple file was done, with this file being created on the fly for each test.

This commit replaces that check with running the "explain" command.

Advantages:

  • The "explain" command does not need a file to scan and can still confirm that the standard can be read correctly by PHPCS.
  • The output for the "explain" command has fewer differences across PHPCS versions, so is simpler to verify.

🆕 BaseLineTest: selectively skip the tests

... in a very specific combination of circumstances - Windows + Composer 1.x + PHP 5.5 + PHPCS dev-master + no external standards -, as the Composer logs in that case, don't always show the "Running PHPCodeSniffer Composer Installer" message.

As it is such a specific combination, which will probably be rare to encounter in real life, I'm proposing to skip the tests for that combination instead of spending lots of time trying to debug this (for now).

🆕 TestCase: add willPluginOutputShow() method

In very select circumstances (PHP 5.5 with Composer 1.x on Windows) and sometimes even only with select PHPCS versions, the plugin output will not be shown in the transcript from Composer, even though the plugin does actually run.

Instead of skipping those tests completely, I'm suggestion to selectively skip the output expectation for the composer install/update run instead.
That way we can still verify that the plugin has actually done its job by checking that paths have been registered with PHPCS.

I would recommend to only implement the use of this method for those select tests where we have actually seen failures due to this Composer bug and to just leave the other tests alone.

Related Issues

Closes #9

@jrfnl
Copy link
Member Author

jrfnl commented Feb 1, 2022

Note: the test failure on PHP 5.5 with Composer v1 on Windows is a really weird one. It doesn't fail on every run, just most of the time and it only seems to fail for PHPCS dev-master, which makes this even weirder. I've been trying to figure out the cause, but haven't been able to get to the bottom of it so far.

As it is a very specific combination of circumstances and one which will probably be rare IRL nowadays, we may need to decide to just skip the test for that specific combination.

@jrfnl jrfnl force-pushed the feature/add-initial-integration-tests branch from 86d0c60 to 587b1bc Compare February 1, 2022 08:20
@Potherca
Copy link
Member

Potherca commented Feb 1, 2022

Review planned on: 2022-02-04.

@jrfnl
Copy link
Member Author

jrfnl commented Feb 2, 2022

Note: the test failure on PHP 5.5 with Composer v1 on Windows is a really weird one. It doesn't fail on every run, just most of the time and it only seems to fail for PHPCS dev-master, which makes this even weirder. I've been trying to figure out the cause, but haven't been able to get to the bottom of it so far.

As it is a very specific combination of circumstances and one which will probably be rare IRL nowadays, we may need to decide to just skip the test for that specific combination.

I've added a commit to skip the test which fails intermittently for now.

@jrfnl jrfnl force-pushed the feature/add-initial-integration-tests branch 2 times, most recently from 73388f2 to b973ddf Compare February 2, 2022 08:08
@jrfnl
Copy link
Member Author

jrfnl commented Feb 2, 2022

Note: to make this PR mergable, the required checks for the branch (branch protection settings) will need updating as well.

@jrfnl
Copy link
Member Author

jrfnl commented Feb 4, 2022

I've made a minor update to the PHPCSVersions class to ensure it is in-line with the update I made to PR #152.

@jrfnl jrfnl force-pushed the feature/add-initial-integration-tests branch 2 times, most recently from 60ef347 to 0ed5d64 Compare February 4, 2022 13:15
@jrfnl
Copy link
Member Author

jrfnl commented Feb 4, 2022

Rebased without changes to get it past the merge conflicts.

@jrfnl jrfnl force-pushed the feature/add-initial-integration-tests branch from 0ed5d64 to 28fe834 Compare February 4, 2022 13:37
@jrfnl
Copy link
Member Author

jrfnl commented Feb 4, 2022

Oh joy.. the new assignment operator alignment sniff was throwing some issues. Fixed up now in the respective commits. No changes other than those whitespace fixes.

@jrfnl jrfnl force-pushed the feature/add-initial-integration-tests branch from 28fe834 to 1511a24 Compare February 5, 2022 07:16
@jrfnl
Copy link
Member Author

jrfnl commented Feb 5, 2022

FYI: push was to make a tiny formatting tweak to the README for the fixtures, no textual or functional changes.

@jrfnl
Copy link
Member Author

jrfnl commented Feb 5, 2022

Added one more commit to the PR as I found that some of the other tests I'm working on will also need to selectively skip a certain output expectation. Abstracting the condition to a method will make it more easily re-usable in those other tests.

Potherca
Potherca previously approved these changes Feb 6, 2022
Copy link
Member

@Potherca Potherca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got some questions and some pointers but nothing that would stop this from being merged.

Once I'd reviewed the bootstrap and testcase class, the actual test were easy to read/understand. 👍

.gitattributes Show resolved Hide resolved
.github/workflows/integrationtest.yml Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
tests/CreateComposerZipArtifacts.php Outdated Show resolved Hide resolved
tests/IntegrationTest/BaseLineTest.php Outdated Show resolved Hide resolved
tests/IntegrationTest/BaseLineTest.php Show resolved Hide resolved
tests/fixtures/README.md Show resolved Hide resolved
@jrfnl
Copy link
Member Author

jrfnl commented Feb 6, 2022

Thanks for the extensive review @Potherca ! I've left replies everywhere relevant and adjusted those things for which there is no discussion.

Potherca
Potherca previously approved these changes Feb 6, 2022
tests/TestCase.php Show resolved Hide resolved
@Potherca
Copy link
Member

Potherca commented Feb 7, 2022

As all my points have been addressed and all questions have been answered, this is good to merge if you are content with it. 👍

Does #147 have to merged first?

@jrfnl
Copy link
Member Author

jrfnl commented Feb 7, 2022

As all my points have been addressed and all questions have been answered, this is good to merge if you are content with it. 👍

I am. ✔️

I do have some follow-up PRs with tweaks, but didn't want to make the review of this PR harder than it would already be, so I will pull those changes + additional tests in separate PRs once this PR has been merged.

You can expect some ~15 follow-up PRs based on what I have lined up locally so far.

Does #147 have to merged first?

It does. As 0.7.2 has been released now, I've moved that one from draft to "ready for review". Once #147 has been merged, I'll mark this as ready (and if needs be rebase it without changes to get past potential imaginary merge conflicts).

The branch protection settings will need updating for both PRs, so may be easiest to just do that once after this one has been merged.

@mjrider Did you still want to have a look at either of these PRs before we merge them ?

* Require the PHPUnit Polyfills as a basis for the tests.
    The PHPUnit Polyfills will ensure a compatible PHPUnit version will be available and will allow for writing tests for the latest version of PHPUnit (9.x), while still being able to run the tests on PHPUnit 4-8.
* Include a basic test bootstrap file which will allow for running the tests both via a Composer installed version of PHPUnit as well as via a PHPUnit PHAR file.
    Note: the bootstrap file will try to determine where the `composer.phar` file is located. For running the tests locally and/or overruling the default version of Composer, a `COMPOSER_PHAR` environment variable can be set in a local `phpunit.xml` overload file.
* Include information about the test setup in the `CONTRIBUTING` documentation.
* Includes adding new jobs to the "Quick test" and "Integration test" GH Actions workflows.
    Once the existing tests in the _old_ workflows have been converted to the new test setup, the old workflows can be removed.
* Minor tweaks to various repository config files.

Note: this test setup presumes that the proposal to drop support for PHP 5.3 - see issue 145 - will be accepted as the PHPUnit Polyfills require PHP >= 5.4.

Refs:
* https://github.com/Yoast/PHPUnit-Polyfills/
jrfnl added 13 commits March 5, 2022 11:09
As most tests will need to run a `composer install`, tests should be run in separate, temporary directories.

With that in mind, I'm adding two helper methods to the base `TestCase` class:
* `createTestEnvironment()` - this method will create a `PHPCSPluginTest/TestClassName_uniqueID/local` and a `PHPCSPluginTest/TestClassName_uniqueID/global` directory in the system temp directory and set the Composer global `HOME` directory to the created `global` directory.
* `removeTestEnvironment()` - this method will remove the created directories and reset the Composer global `HOME` directory to its original location.

Notes:
* The paths to the created directories will be added to the `$tempDir`, `$tempGlobalPath` and `$tempLocalPath` properties of the class and can be used in the tests.
* The methods (and associated properties) have been made `static` on purpose to allow for these methods to be used both in `setUp()`/`tearDown()` fixtures as well as in `setUpBeforeClass()`/`tearDownAfterClass()` fixtures.
    The latter would be useful for a test class needing only one environment with various test methods depending on each other and building on each other.
    This is also the reason that these methods are set up as helpers instead of as fixtures as this way, the decision what fixtures are most appropriate can be made for each individual concrete test class.
* A hard cleanup of the `PHPCSPluginTest` directory in the system temp directory has been added to the test bootstrap to make sure that any stray "old" test directories will be removed before a new test run.

Includes a `static` `onWindows()` helper method to determine whether or not the tests are being run on Windows.
This adds a test helper method, which, given an array with a Composer configuration and a target location, will on-the-fly create a `composer.json` file for use in a test.

The method will automatically add two additional settings to the `composer.json` file:
* It will add a `install-codestandards` script to run the plugin on demand.
* It will add the `allow-plugins` permission, which is needed for Composer 2.2.

By adding these keys automatically, this kind of "noise" is not needed in the actual test classes.

The method is `static` so as to allow for it to be used in `setUpBeforeClass()`/`tearDownAfterClass()` fixtures.
In normal circumstances, a `composer install` will download a version of the plugin from Packagist.
This will not work for the tests however, as the development version of the plugin being tested will generally not be available on Packagist.

With this in mind, we need a work-around to ensure that the tests are run with the **current** version of the plugin as per the last commit in the current branch.

I've implemented this by using the `repositories - artifact` feature of Composer.

This works as follow:
* In the test bootstrap, ahead of the test run, the plugin is zipped up as a package and placed in the `tests/artifact` directory.
* Whenever a `composer install/update` command is run within the test suite, the `repositories-artifact` key is injected into the `composer.json` when the `composer.json` file is written to the temp directory used for the test.
* With this in place, Composer will install the plugin from the zipped file in the `tests/artifact` directory instead of downloading from Packagist.

Notes:
* The version number to be used for the zip artifact of the plugin is defined in the test bootstrap file.
    This number may need updating once in a while to ensure it is higher than the latest released version of the plugin.
* In the `CreateComposerZipArtifacts` class, three properties are used to determine which files to exclude from the package - `$excludedFiles`, `$excludedExtensions` and `$excludedDirs`.
    Depending on new files/directories being introduced in the package, these lists may need to be updated once in a while.

Ref: https://getcomposer.org/doc/05-repositories.md#artifact
This commit adds six additional methods to the base `TestCase` class:
* `executeCliCommand()`: a helper method to execute a CLI command and return the exit code, the contents of `stdout` and the content of `stderr` for use in assertions.
* `stabilizeCommand()`: a helper method which is automatically called from the `executeCliCommand()` method. This method will:
    - Ensure that the PHP version which was used to initiate the test run will also be used in the CLI command.
        Without this, the system default PHP version would be used for CLI commands, which may be a different version from the PHP version used to run the tests and would skew the test results.
    - Ensure that the Composer version as set up using the `env` variable is used in the tests.
        Again, this will prevent the system default version of Composer being used, which may skew the results when the tests are intended to be run against a specific version of Composer.
    - Adds the `--no-interaction` flag to all Composer CLI commands to prevent tests hanging when interaction would be expected.
    - Adds quotes around a command when the tests are run on Windows in combination with PHP < 8 to ensure cross-platform compatibility of the CLI commands.
* `getPhpcsCommand()`: a helper method to get the path to the vendor installed PHPCS entry point file.
* `maybeStripColors()`: a helper method to compare CLI output. **This method is still WIP and will probably still need some tweaking.**
    This method will take an output expectation and an actual output and will attempt to strip CLI colour codes from the actual output when the output expectation does not contain colour codes. This way, expected colour codes can still be tested by including them in the output expectation.
    Note: until the bugs in this method have been smoothed out, it is recommended to use `--no-ansi` CLI arguments in CLI commands not testing for colour codes.
* `assertExecute()`: a custom assertion which will run a given CLI command, optionally in a specific working directory and will subsequently assert that the exit code is the same as expected and the `stdout` and `stderr` outputs contain the expected output.
    For more complex assertions against the output of CLI commands, the `executeCliCommand()` can be used directly in tests.
    The `StringContainsString` assertions used for `stdout` and `stderr` will attempt to strip color codes from the output before doing a compare using the `maybeStripColors()` method.
* `assertComposerValidates()`: a custom assertion to verify that a given `composer.json` file validates for use in the tests.

A number of these methods have been made `static` to allow for them to be used in conjunction with `setUpBeforeClass()` fixtures.
While not necessarily the most pretty solution (output is displayed in the middle of the progress report), this allows for access to the `composer.json` content, the CLI commands and their output for any test which failed, which will be helpful when debugging test failures, especially in CI.
These tests verify:
- That the plugin can be installed and functions correctly with the full range of supported PHPCS versions.
    While the test is not run against the full range of supported PHPCS version each time, due to the random PHPCS version selection, all supported PHPCS versions will be tested over time in CI.
- That the plugin runs when installed.
- That the PHPCS native standards are the only recognized standards when no external standards are available.
- That no `CodeSniffer.conf``file gets created when no external standards are found.

These tests will be run in both a Composer global as well as a Composer project local environment.

The tests use a dataprovider which will randomly select two PHPCS versions compatible with the PHP version on which the test is being run + PHPCS `dev-master` + PHPCS `4.x` (dev version for the next major).

As which PHPCS versions are compatible with which PHP versions needs quite some logic, a separate helper class `PHPCSVersions` is introduced, which encapsulates that knowledge.

The `PHPCSVersions` class contains a number of (`static`) methods as helpers. These can be used in any test class:
* `get()`: retrieves an array with a specific number of PHPCS versions valid for the current PHP version.
* `getHighLow()`: retrieves an array of the highest and lowest PHPCS versions valid for the current PHP version.
* `getHighLowEachMajor()`: retrieves an array of the highest and lowest supported PHPCS versions for each PHPCS major (and valid for the current PHP version).
* `getRandom()`: gets a random PHPCS version which is valid for the current PHP version.
* `toDataprovider()`: converts a version array to an array suitable for use as a PHPUnit dataprovider.
* `getSupportedVersions()`: retrieves an array with all PHPCS versions valid for the current PHP version.
* `isNextMajorSupported()`: determines whether the current PHP version is supported on the "next major" branch of PHPCS.
* `getStandards()`: retrieves an array of the names of the PHPCS native standards for a particular PHPCS version.

Generally speaking, the `dev-master` and `4.x` branch will not be included in the retrieved ranges, but can be specifically requested by passing optional parameters.

This `PHPCSVersions` class will need semi-regular updates when new versions of PHPCS are released and new PHP versions are released.
... in a very specific combination of circumstances - Windows + Composer 1.x + PHP 5.5 + PHPCS `dev-master` + no external standards -, as the Composer logs in that case, don't always show the "Running PHPCodeSniffer Composer Installer" message.

As it is such a specific combination, which will probably be rare to encounter in real life, I'm proposing to skip the tests for that combination instead of spending lots of time trying to debug this (for now).
This moves the test which was being run via GH Actions to a test class in the integration test suite.

Differences between the previous and new setup:
* The previous test via GH Actions would only fail on the exit code not being `0` for any of the steps.
    The new PHPUnit based tests executes more detailed assertions on the results of the commands being run in the tests.
* The previous test would execute the test run of PHPCS against the `src/Plugin.php` file in this package.
    The new PHPUnit based tests will execute against a simple PHP file created on the fly.
* The previous test via GH Actions would only run for a Composer `local` situation.
    The new PHPUnit based tests cover both a Composer `global` as well as a `local` setup.
* The matrix against which the tests are run is different.
    In the old situation, the test would only be run on a fixed set of PHP/PHPCS combinations with Composer 2.x on Ubuntu.
    In the new situation, the tests will run against both Composer 1.x, as well as 2.x on both Ubuntu and Windows.
    Same as before, it will be run against all supported PHP versions (in the extended `integrationtest` workflow), but the PHPCS versions to test against are more varied and - in part - randomly selected out of the PHPCS version compatible with the current PHP version, which results in more comprehensive testing of these scenarios.

Includes:
* Adding a new `createFile()` helper method to the base `TestCase` to generate a (PHP) file to do a test run against.
* Removing the "old" integration test from the GH Actions script.
…ct of it

The tests in the `RegisterExternalStandardsTest` class contain quite some work-arounds to handle changes made over time to the external PHPCompatibility standard.

The `PHPCompatibility` standard was chosen for use in the tests as it is one of the rare standards which still supports both PHPCS 2.x as well as 3.x.

All the same, to improve the stability of the tests and to simplify the actual test code, using "fake" PHPCS standards set up as fixtures as part of this test suite will make life easier.

This commit creates the initial setup for that, by:
* Adding code to the `CreateComposerZipArtifacts` class to handle creating Composer package zip artifacts for each subdirectory of the `tests/fixtures` directory.
* Calling that code from the test bootstrap file to ensure all test artifacts are zipped up and ready for use ahead of running the tests.
* Adding an initial "fake" `DummySubDir` standard as a test fixture.
* Adding a `README` file to the `tests/fixtures` directory with the rationale and a list of available fixtures.
…st fixture

This replaces the use of the PHPCompatibility standard in the `RegisterExternalStandardsTest` tests with the use of the `dummy-subdir` fake standard fixture.
…ith the standard

To check whether PHPCS can run with the standard, previously, a scan on a simple file was done, with this file being created on the fly for each test.

This commit replaces that check with running the "explain" command.

Advantages:
* The "explain" command does not need a file to scan and can still confirm that the standard can be read correctly by PHPCS.
* The output for the "explain" command has fewer differences across PHPCS versions, so is simpler to verify.
In very select circumstances (PHP 5.5 with Composer 1.x on Windows) and sometimes even only with select PHPCS versions, the plugin output will not be shown in the transcript from Composer, even though the plugin _does_ actually run.

Instead of skipping those tests completely, I'm suggestion to selectively skip the output expectation for the `composer install/update` run instead.
That way we can still verify that the plugin _has_ actually done its job by checking that paths have been registered with PHPCS.

I would recommend to only implement the use of this method for those select tests where we have actually seen failures due to this Composer bug and to just leave the other tests alone.
@jrfnl
Copy link
Member Author

jrfnl commented Mar 5, 2022

This PR depends on PRs #147 and #152 both being accepted and merged.

Rebased now the prerequisites have both been merged & marking this ready for review/merge.

@jrfnl jrfnl marked this pull request as ready for review March 5, 2022 10:11
@jrfnl
Copy link
Member Author

jrfnl commented Mar 5, 2022

Note: to make this PR mergable, the required checks for the branch (branch protection settings) will need updating as well.

I've updated the branch protection settings to match the new builds and removed the old builds. PR should be mergable now.

@Potherca Potherca merged commit 8ab9bb5 into master Mar 5, 2022
@Potherca Potherca deleted the feature/add-initial-integration-tests branch March 5, 2022 12:36
@jrfnl
Copy link
Member Author

jrfnl commented Mar 5, 2022

Thanks for getting this merged. I'll start pulling the follow up PRs.

@jrfnl
Copy link
Member Author

jrfnl commented Mar 5, 2022

Related to #92

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.

Add PHPUnit tests and CI
2 participants