Iterate further on the integration tests via Travis

[jrfnl]

This ticket is meant as the next iteration, following up on #91

In this iteration, the following additional variants should be explored:

• Having some builds with a global install of PHPCS + global install of the external standard used for testing.
• Having some builds with a mix of global/local installs of external standards(s).
• Having some builds using Windows as OS instead of Linux.

[Potherca]

A couple of years ago I create some code for phpcodesniffer-composer-installer issue-24 that might be usable for some of this.

[Potherca]

Not sure how/where to best run build steps in Windows. Maybe @mjrider has some ideas?

[jrfnl]

@Potherca I know you can set the OS for specific builds in both Travis as well as actions, so either could work.

[jrfnl]

Especially with an eye on preventing regressions, while still broadening support to newer versions, like PHPCS 4.x, PHP 8.x and Composer 2.x, I've made an inventory of test cases which would need to be included, by going through every single issue and PR in this repo from the very beginning to the time of writing.

My focus has been on integration tests and I have made "shorthand" notes.
I don't have time at this moment to get this setup completely, nor have I a clear idea on how we can set up these type of CLI integration tests easily.

All the same, I figured it may be useful to make these notes public for whomever starts work on getting tests set up when they have time.

---

OS matrix

• Windows
• Unix

Travis matrix:

• PHPCS 2 low/high - PHP 5.3 - PHP 8
• PHPCS 3 low/high - PHP 5.4 - PHP 8
• PHPCS 4 (allowed to fail) - PHP 7.2 - PHP 8
• Composer 1
• Composer 2

Possibly use:
--prefer-lowest (works with update/require)
--prefer-stable (works with update/require)
--prefer-source (works with update/require/install)

Basic test scenarios:

• Local install
• Global install

More detailed test scenarios:

• With another dependency which has a file autoload (Symfony polyfill)
• In combination with other Composer plugins
• In combination with post-install/update-cmd's (see Bugfix: Composer scripts/commands broken in 0.6.0 update #105)

Note: When error output is expected, test with both --verbose and without and verify the output.

Caveats:
As the tests would include tests which change the global composer install, running tests locally should be done in a VM/Docker container.

On top of that, it would be a good idea to make a backup of the Composer global composer.json, composer.lock and phpcodesniffer.conf files before running the tests and put those back afterwards (and running a composer install to resync the global Composer setup to what it was before running the tests)

Rough idea about basic test setup:

• create-project test
• fill in initial setup for test situation
• if different results are expected for PHPCS 2/3/4, use phpcs --version to retrieve the version number and set the $expected results based on that.
• run test
• remove the test project

=> figure out how to use the "current" branch of the plugin in the tests as it may not (yet) be public

Things to test:

• catch the output of the composer command and check the bit coming from this plugin is displayed
• run phpcs -i
• run phpcs --config-show

composer install with composer.json with

• no external standards
• one external standard
• three different external standards

composer require & require-dev

• adding one external standard
• adding two external standards
• removing one external standard
• removing two external standards

composer require/remove this plugin

• without PHPCS installed
• with PHPCS installed, no external standards
• with PHPCS installed, multiple external standards, paths set already relative paths
• with PHPCS installed, multiple external standards, paths set already absolute paths

other

• confirm it doesn't run when using --no-scripts
• confirm it doesn't run when using --no-plugins
• confirm running "Dealerdirect\Composer\Plugin\Installers\PHPCodeSniffer\Plugin::run" directly then works

test with installing something with --ignore-platform-reqs where one of the other dependencies of the project has a higher PHP requirement

test situation where the root project is a PHPCS standard which requires the plugin

• make sure it only sets this path if the root package contains phpcodesniffer-standard as type & contains a ruleset.xml file in the root ?

test with different search depths set via extra

{
    "extra": {
        "phpcodesniffer-search-depth": 5
    }
}

test with different search depths set via composer config extra.phpcodesniffer-search-depth 4

-- see if we can set up a test which triggers other Composer command steps to verify this plugin does not interfere
See: #48, #49, #53

test with the plugin installed with different versions locally and globally

• See: https://github.com/Dealerdirect/phpcodesniffer-composer-installer#caveats and https://getcomposer.org/doc/articles/plugins.md#using-plugins
• Issue Plugin not working correctly when sniffs install depth is equal to "1" #13
• local old / global new
• global old / local new
• what about when PHPCS is installed both globally and locally ?

test with other Composer PHPCS plugin installed as well

• do they play nice together ?
  -> or should a new feature be added to detect other plugins and alert on them ?

Candidates:

• https://packagist.org/explore/?query=php_codesniffer&type=composer-plugin
• higidi/composer-phpcodesniffer-standards-plugin (abandoned in favour of this plugin)
• simplyadmire/composer-plugins (also abandoned, no alternative mentioned)

Issue #13

• install older PHPCS version
• run composer update
• see if it all worked & the path is set for the new PHPCS version

Issue #32

• mix of absolute and relative paths in installed_paths including duplicates (once as absolute, once as relative)
• run the plugin
  -> verify the output is correct and contains all paths only once

Issue #33

• PHPCS 2.x: Make sure that path is always set to the directory above the directory containing ruleset.xml and potentially the /Sniffs/ directory as otherwise the path won't be compatible with PHPCS 2
• PHPCS 3.x: Verify the path is set to the standards directory.

Issue #63

• Test with standard installed in non-vendor directory, but somewhere under the root of the project anyway.

Issue #103

• do a full install with PHPCS, the plugin + external standards
• --no-dev install (uninstall the PHPCS + plugin packages)
• make sure that nothing breaks (no exit code 1)
  plugin will still run even when uninstalled as it uses the pre-uninstall state and has been loaded in memory already

test without PHPCS installed
i.e. "manually" removed it from the vendor directory and verify that the plugin gives an appropriate error message

External standards types to test with:

• standards with "default" directory layout, i.e. root/StandardName/Sniffs
• standard with additional PHPCS dependency (PHPCSExtra -> PHPCSUtils)
• standard with external dependency (Slevomat -> PHPParser)
• standard with sniffs directory at top level of the package, i.e. root/Sniffs (old PHPCompatibility could be used - make sure path is set to dir above for compatibility with PHPCS 2)
• standard with deeper directory structure, i.e. root/Sub/StandardName/Sniffs (DrupalCoder/ObjectCalisthenics - make sure path is set to "Sub" for compatibility with PHPCS 2)
• standard with multiple standards (WPCS/DrupalCoder/PHPCSExtra)
• standard without sniffs, just ruleset.xml (Doctrine = also in /lib/ subdirectory or PHPCSUtils)
• standard where this plugin is a dependency of the standard (PHPCSExtra)
• standard which doesn't have the phpcodesniffer-standard type set (mamis - shouldn't be installed)

wp-coding-standards/wpcs
slevomat/coding-standard
phpcompatibility/php-compatibility
object-calisthenics/phpcs-calisthenics-rules
pheromone/phpcs-security-audit
sirbrillig/phpcs-variable-analysis
drupal/coder
doctrine/coding-standard
phpcsstandards/phpcsutils
phpcsstandards/phpcsextra
mamis/phpcs-coding-standard

[Potherca]

That's a really nice overview to have. Very worthwhile!

I've found the test code I wrote for #103 and the test code for #105 .

We might want to make separate tickets from the shorthand notes so that each topics/sections can be worked on individually.

I think one of the first things to do is to set up "something" generic to make it easier to write other tests on top of.
At the moment I have no idea when I'll have time to create something...

[Potherca]

By the way, I concur with the suggested setup.

The testcode originally written for #24 has a similar setup.

Specifically

• Local install vs. Global install

• In combination with other Composer plugins

• In combination with post-install/update-cmd's (see Bugfix: Composer scripts/commands broken in 0.6.0 update #105)

Although written in/for PHPUnit, the test script calls Composer on the commandline (instead of calling the PHP code directly, as the unit-tests do).

The test code written in BASH was good to get things up and running quickly but the PHP code is better for the long-term as it is much easier to read, re-use, maintain and extend.

Running the PHP-code test with --verbose would be trivial, probably just extend the original class and extend the executeComposerCommand function to accept an extra CLI flag.

make a backup of the Composer global composer.json, composer.lock and phpcodesniffer.conf files before running the tests and put those back afterwards

The first two files are taken care of, the last could be added. They are backed-up once (with setUpBeforeClass) and restored after each test (with tearDown).

figure out how to use the "current" branch of the plugin in the tests as it may not (yet) be public

Assuming we want the tests live in this repo (maybe in tests/integration/ ?), the tests should run on the checked out branch. I can't remember exactly how I did that but I know I've got something for that somewhere.

The output is caught by running commands through exec: exec($command, $output, $exitCode);

Several (I think most, except three of four) of the composer commands to run would be quite easy to implement. The phpcs commands would be better served by another/separate test-file, i think.

Definitely interesting stuff I would like to work on. Earliest available time currently looks like September after the holidays, when my work with PDS Interop is further along, and when there is less Corona craziness.

[jrfnl]

Just came across this Composer plugin which may be useful when setting up the integration tests: https://packagist.org/packages/g1a/composer-test-scenarios

[Potherca]

@jrfnl Nice find! I'll have a look at it when my schedule allows. 👍

[Potherca]

Regarding my schedule: After September, my time was mostly taken up with my work for @pdsinterop and getting everything finished before specific deadlines.

Currently, I'm involved in a project for the Dutch Government implementing COVID-19 related software, for which there is a lot of pressure. So until things settle down, I won't be able to contribute much here (as you have probably already noticed). 😓

[jrfnl]

@Potherca Appreciate the update.

To be honest, I think between COVID and work pressures, everyone is at, or near, the end of their energy. Also see this blogpost I wrote about this and and other things a while back: https://24daysindecember.net/2020/12/21/a-perfect-storm/

As for my part:
Between wrangling PHPCS and external standards for it to support all the new PHP 8 syntaxes and getting tests for lots of projects running on PHP 8, I've barely had time to breath myself and PHP 8.1 is already around the corner, what with the first alpha having been released last week.

I'm still hoping to release the 1.0.0 version of PHPCSUtils soon (was due end of last summer) and PHPCompatibility 10.0.0, WPCS 3.0.0 and PHPCSExtra 1.0.0 will follow soon after and will all include a non-dev dependency on this project.

Luckily, this repo doesn't need that much attention anyhow as it just works for the most part 😉

Also: we're nearing the 14 million downloads ;-) (last time I looked it was 8 million or something) 🎉

[jrfnl]

FYI: as we touched upon this issue again yesterday, I got seduced into having a go at creating a working and easily extendable setup for this and after a fruitful night, I now have a working proof of concept.

The first test I've created basically moves the "integration test" as was being run via Travis to PHPUnit.

The principle is basically E2E testing.

The setup I've currently got uses an abstract TestCase for the heavy lifting. Concrete test classes can set an initial composer.json for a global install and/or a local install, and then pass an arbitrary set of commands. The commands can use a data provider for variable replacement (like the PHPCS version and such) and you can set expectations for the exitCode, stdOut and stdErr for each command.

How does that sound ?

And is anyone available to do an initial review some time in the foreseeable future ? If the setup can be agreed upon, it should be pretty straight forward to expand on it with additional tests.

[Potherca]

Sounds good! I've (almost) always got time for reviews somewhere during each week. 👍

[jrfnl]

@Potherca Excellent! I'm still trying to work out some kinks and the files need a LOT of clean up, but would appreciate a look. Or even talking some things through/review call.

Current issue, which is only an issue when running locally, shouldn't be an issue in CI: getting the commands to use the current PHP version and not the system default version (as they are in a different process). For this I also need to know the location of the Composer phar file, but for some reason I can't seem to find a Composer CLI command to show the location to me.
Also see: https://twitter.com/jrf_nl/status/1404379377509539840

[jrfnl]

Finally got some time to get back to this again. Looking to clean up and improve on what I set up previously over the next few days.

The current setup is PHPUnit based, all the same, I do keep wavering between different options for "test framework", with expect being a likely alternative. Also looked at using PHPUnit with phpt files and tools like cmdt, shelltestrunner and exactly.

The reason for initially choosing PHPUnit is mostly as contributors to this repo will most likely be relatively familiar with it and it would make it more accessible because of that to add tests and to run tests locally (and in CI on different platforms, which not all other tools are suitable for).
It also allows for setting up numerous helper methods and working with data providers, which allows testing the same situation with, for instance, various different PHPCS versions in a straight-forward manner.

Still needs to figure out this one though:

For this I also need to know the location of the Composer phar file, but for some reason I can't seem to find a Composer CLI command to show the location to me.

[Potherca]

Finally got some time to get back to this again. Looking to clean up and improve on what I set up previously over the next few days.

As luck would have it, I've got a week of work, so I've got some time to look into this in the coming days as wel!

The reason for initially choosing PHPUnit is mostly as contributors to this repo will most likely be relatively familiar with it and it would make it more accessible because of that to add tests and to run tests locally (and in CI on different platforms, which not all other tools are suitable for).

At this point, I think that is a winning argument, so lets keep it at PHPUnit and work out things from there. 👍

[jrfnl]

Status update:

Done

• Initial setup, including converting integration test from GH Actions to PHPUnit E2E test
  Status: PR Add initial integration test setup and first few tests #153
• Test that the plugin does not act on non-PHPCS standards (issue Invalid "installed_paths" config value #32)
  Status: Waiting for PR 153 to be merged first
• Test not breaking a pre-existing PHPCS config (issue Bug fix: loadInstalledPaths() very very broken since PHPCS 3.1.0 #98)
  Status: Waiting for PR 153 to be merged first
• Test with a pre-existing PHPCS config which includes installed_paths outside of the Composer vendor directory
  Status: Waiting for PR 153 to be merged first, needs bugfix related to [2.2.2+] $this->composer->getConfig()->get('bin-dir') returns incorrect path on Windows composer/composer#10504, bugfix ready to be pulled
• Test that the plugin does not prevent post-install/update scripts from running (issue Bugfix: Composer scripts/commands broken in 0.6.0 update #105)
  Status: Waiting for PR 153 to be merged first
• Test that the plugin always registers the paths in the same order (issue Order of installed_paths inconsistent between runs #125)
  Status: Waiting for PR 153 to be merged first
• Test to confirm the plugin runs on select events and doesn't run on any other events, including that it doesn't run when --no-plugins is used.
  Status: Waiting for PR 153 to be merged first
• Test handling of the plugin being installed in a project which is an external standard (issue Coding Standard tries itself to install with installPath when it's the root package #19, Check the root package for sniffs to install #20, Invalid "installed_paths" config value #32).
  Status: Waiting for PR 153 to be merged first
• Test that the plugin doesn't throw errors when PHPCS or the plugin is uninstalled (issue Do not exit with code 1 on uninstall (--no-dev) #103)
  Status: Waiting for PR 153 to be merged first
• Expand tests for registering external standards to include more PHPCS standard setups (like ruleset in project root, ruleset in deeper nested subdir, standard with multiple rulesets, standard which requires a secondary standard etc)
  Status: Sort of ready, waiting for PR 153 to be merged first

WIP:

• Test the search depth feature (@jrfnl)

To do:

Please feel free to volunteer to add any of these tests... 😇

• Test that the plugin handles new standards being added/removed correctly in an existing install
• Expand plugin removal tests with additional situations like:
  • uninstalling the plugin, but PHPCS remains.
  • uninstalling a standard which requires the plugin, but PHPCS remains
  • manually deleting the PHPCS directory from vendor
  • plugin installed both globally and locally and locally is removed
• Test that the plugin plays nice with other Composer plugins
• Test all situations in which debug messages are expected to be displayed
• Test with the plugin installed with different versions locally and globally
• Test with initial install having an older PHPCS, running composer update and confirming that the new PHPCS has the expected installed paths
• Test with the vendor directory having a different name (like in combination with the Composer Installers)
• ...etc...
• ...etc...

[Potherca]

This is a rather long thread and I fear that the conclusion might become buried/overlooked.

@jrfnl Should we close this and create new issue(s) with the actionable conclusion(s)?

[Potherca]

Also, very nice work @jrfnl! I keep being amazed by the quality of your contributions. I don't think thats said often enough...🧢📴¹

Footnotes

1. https://en.wiktionary.org/wiki/take_one%27s_hat_off_to ↩

[jrfnl]

Thanks ;-)

I think the still open items in the action list in my previous comment could be moved to a follow up ticket or even to individual tickets, one for each action item.