We should be able to demonstrate that the test runner handles PHP linting errors properly.
This should be a test that uses a patch to introduce a linting problem into the PHP of Drupal core.
Create a patch that causes a linting error in a changed file. Use php -l on the patched file to discover whether there is an error.
Add the patch to this issue. That way the patch will be around forever.
Subclass DrupalCIFunctionalTestBase to write a test which applies the patch. Follow the example of DrupalCI\Tests\Application\CorePatchAppliedTest to create this test.
Follow-up to create a similar test for contrib.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2809565_actually_fails_linting.patch | 186 bytes | Mixologic |
| #10 | 2809565_will_totally_fail_linting.patch | 213 bytes | Mixologic |
| #3 | 2809565_will_fail_linting.patch | 205 bytes | mile23 |
Comments
Comment #2
mile23Comment #3
mile23A patch to fail linting.
Comment #5
mile23I'd be super awesome if there were an example of this in the wild. So far I can't make the runner fail linting. I'd wager the fault is in
Patch::getModifiedFiles(), or else I'm doing something wrong.Comment #6
mile23Note I'm definitely not assigned to this issue. Please patch dev as desired. :-)
Comment #7
legovaerI thought that the scope for this issue was a D.O. module that contains a syntax error. Therefore, I've created drupalci_d8_module_snytax_error
Comment #8
legovaerAs I said before, the scope is not 100% clear. But: I've created a patch for an existing module which introduced a syntax error:
https://www.drupal.org/pift-ci-job/488073
Comment #9
mile23Updated IS.
The testbot figures out which files have been patched, and then lints only those files using
php -l. So in order to find out if the testbot does the right thing when it encounters an error like that, we have to patch the error into the code.Comment #10
MixologicTwo probs with that fail patch. One, we only lint php files, which we determine by looking for <?php at the start of a fail, since this didnt match that pattern, it never thought it should lint it.
Second was that we were only doing a git diff --name-only to show modified files. Which only shows modified files, and not new files. So I fixed that also, elsewhere.
Comment #11
MixologicSurprisingly difficult to get the right kind of phplinting failure.
Comment #12
MixologicThis was done a while ago.
Comment #13
Mixologic