D.O. test runners fail - incompatible drupal/coder version constraint

For the record, in 32e1f06 and 31ce2d4 we set "drupal/coder": "8.2.*" as a dev requirement in rules composer.json

So are you saying that the d.o. testbot uses coder: 2.* to actually mean 8.2.* because it already knows the core version is 8 ?

I have just noticed that in a Travis build, when executing travis_retry composer self-update && travis_retry composer install it already installs drupal/coder (8.2.12). So I tried a Travis build without drupal/coder in composer.json (and also without it being installed directly from github like we used to have). Not having this in composer.json should allow the d.o. testbots to run as before (?maybe).

However, when executing cd $DRUPAL_ROOT/modules/$MODULE && ./vendor/bin/phpcs -e --report-width=130 I got the error

PHP Fatal error:  Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff "Drupal.NamingConventions.ValidVariableName.LowerCamelName" does not exist' in /home/travis/build/jonathan1055/drupal/modules/rules/vendor/squizlabs/php_codesniffer/CodeSniffer.php:1167

This is the sniff we reference in phpcs.xml.dist. Hence, running without drupal/coder in our composer.json might be possible in Travis but it would need something to fix the standard being used.

I tried explicitly using the Drupal standard ./vendor/bin/phpcs -e --report-width=130 --standard=Drupal as got the following

ERROR: the "Drupal" coding standard is not installed. The installed coding standards are PEAR, Zend, MySource, PSR2, Squiz, PHPCS and PSR1

Maybe we can add that standard using the phpcs command to set 'installed_paths' to /drupal/coder/coder_sniffer/

I just had a light-bulb moment, when thinking about why d.o. used "2.*" and I wondered if it was just taking the last part of "8.2.*" such that if we coded "8.*" it might work. I tried this in a Travis build ran sucessfully. So here is a patch to test on d.o. I am not sure if this will work, because there have been issues around when a patch alters the composer.json file whether the new file is used before the old unpatched version. So if this test fails to load, it might be a case of committing the change anyway, then seeing if it still fails.

Anyway, lets see what happens ...

Jonathan

[edit: even though I set the status to 'needs review' and selected 'test with ..' the patch did not automatically queue to be tested. I had to queue it manually. Don't know why]

As I suspected, the composer.json file is used before it is patched. The error is the same as the committed code:

- The requested package drupal/coder 2.* exists as drupal/coder[8.2.12] but these are rejected by your constraint.

However, there is no '2.*' in the patched composer.json file. This is a bit of an issue with d.o. test environment, because it means you cannot properly test changes to that file.

@fago, I have a good hunch that this patch will solve our problem. Do you want to gamble and commit it on d.o. as that is the only way we will actually know. You probably want a PR before that, so I will create a branch and make one.

The PR is https://github.com/fago/rules/pull/492

The Travis build is https://travis-ci.org/jonathan1055/rules/builds/282335824

In #2692407: Test_dependencies are downloaded before applying patches, rather than after any changes to the composer.json file are supposed to be recognised and acted on during the drupal testbot build. But I guess that the fault in our file cause the build to halt before it gets a chance to rectify itself.

The version 2.* most likely stems from the package provided by packages.drupal.org

You might be right. However, here is my reasoning for trying 8.* in composer.json. The failing command

./bin/composer require --ignore-platform-reqs 'dealerdirect/phpcodesniffer-composer-installer:dev-master' 'drupal/coder:2.*' 'squizlabs/php_codesniffer:2.9.*' --ignore-platform-reqs --prefer-stable --no-progress --no-suggest --working-dir /var/lib/drupalci/workspace/jenkins-drupal_contrib-95290/source

appears to me like it is generated from the composer.json dev requirements:

  "require-dev": {
    "dealerdirect/phpcodesniffer-composer-installer": "dev-master",
    "drupal/coder": "8.2.*",
    "squizlabs/php_codesniffer": "2.9.*"
  }

I do not know why the '8' is ignored and the version resolves to '2.*' but if we tried using '8.*' this would give us very useful information, whether it works or if it fails with the same or a different error, we will have concrete knowledge on which to base the next decision on how to resolve this. The testing on drupal.org is currently broken, so committing this change cannot make things any worse. If this commit does not fix the problem I will make a PR and patch to add Coder directly in .travis.yml like we had before, so that we can get d.o. jenkins testing running again. Then we can decide in due course how to change composer.json etc but it will not be holding up other concurrent development on the Rules issue queue.

Jonathan

Having thought about this more, the best way forward from here is actually to remove the recently added "require-dev" section in composer.json immediately, as this change will allow Jenkins test builds on D.O. to start running again. Coder and phpcs are already installed in the Travis build (via the main composer install) so I have used that instead, for Travis.

The pull request is https://github.com/fago/rules/pull/493
The Travis build results for this change are https://travis-ci.org/jonathan1055/rules/builds/288167734
I have added the patch here, but not set it for testing given that testing is broken here anyway, so it will not run.

Then when testing is running again, we can look at having require-dev if we decide it is necessary, and future changes to composer.json will be testsed on D.O. before they are committed.

Good news. The Rules daily tests are now running and passing again - https://www.drupal.org/pift-ci-job/789071

I think there must have been a fix implemented on D.O. testing infrastructure as there have been no commits to Rules since 30th September (which caused the problem we first saw on 1st Oct). The tests failed daily up to 12th October, then started running OK again from 13th October. So our composer.json version definition "drupal/coder": "8.2.*" was fine all along (or so it appears)