search

PHPCSStandards / composer-installer

Composer installer for PHP_CodeSniffer coding standards
https://packagist.org/packages/dealerdirect/phpcodesniffer-composer-installer
MIT License
560 stars 37 forks source link

Iterate further on the integration tests via Travis #92

Open jrfnl opened 4 years ago

jrfnl commented 4 years ago

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

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

Potherca commented 4 years ago

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

Potherca commented 4 years ago

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

jrfnl commented 4 years ago

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

jrfnl commented 4 years ago

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

Travis matrix:

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

Basic test scenarios:

More detailed test scenarios:

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:

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

Things to test:

composer install with composer.json with

composer require & require-dev

composer require/remove this plugin

other

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

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

test with other Composer PHPCS plugin installed as well

Candidates:

Issue #13

Issue #32

Issue #33

Issue #63

Issue #103

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:

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 commented 4 years ago

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 commented 4 years ago

By the way, I concur with the suggested setup.

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

Specifically

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 commented 4 years ago

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 commented 4 years ago

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

Potherca commented 3 years ago

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

jrfnl commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

jrfnl commented 3 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

jrfnl commented 2 years ago

Status update:

Done

WIP:

To do:

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

Potherca commented 6 months ago

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 commented 6 months ago

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

jrfnl commented 6 months ago

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.

Potherca commented 2 months ago

@jrfnl Shall I create a new issue from the mentioned comment?

jrfnl commented 2 months ago

Let's just leave the ticket as-is. It will give anyone who wants to work on expanding the tests all the necessary context without them having to search through multiple issues.