drush coder-review sometime considers the summary as an error, triggering "Drush command terminated abnormally"

Thie idea here was to give an exit code that jenkins, or git, could react to when there is an error. I haven't applied or tested the patch. But we do need to exit differently when there are valid errors. Have you tested/printed what the exit code is when there is an error and what it is when there isn't an error? from the shell you can print $?.

"Drush's exit code has nothing to do with the number of coding errors." is true, but Coder's exit code does have something to do with the existence of errors. When git integration was added, we needed to return this in the exit code thus we needed _coder_review_exit instead of drush_exit to actually return the exit code, because as you point out, drush_exit doesn't do this.

My build fail as well while there is just warnings... I deactivated this module as it is now unusable for our team.

@Sylvain, "I deactivated this module as it is now unusable for our team" makes me think that you have a "team" of people who could make coder better or fix this problem. Is this unusable because of the exit code? You can't make Jenkins ignore the exit code? You can't wrap it in a shell script? Are you running drush as a plugin? A recent fix to the plugin/module issues might of solved this.

Had the same problem with Jenkins and Phing, after upgrading coder to 7.x-2.0.

because I only want to get the code analysis result without breaking the build I looked into the code of the related tools:
The best way I found is to change the build.xml for phing to ignore errors:

<drush command="${coder.review.command}" assume="yes"
       haltonerror="false" pipe="yes" returnProperty="xml">

The haltonerror="false" does the magic!

After changing this I thought about getting a different return code from coders drush task if there are critical issues.
But drush only returns "1" if the drush-command terminated abnormaly, without respecting exit-codes of the actual command. So this would cause patches in drush, coder and DrushTask.php for phing...
Than I drank another coffee to realize that this isn't needed at all, because Jenkins can break the build according to checkstyle-results, which is configurable ("Break if there are more than 5 criticial issues or more than 25 warning...")

---

Info: My Jenkins/Phing build file is based upon this:
https://github.com/reload/phing-drupal-template
which uses
https://github.com/kasperg/phing-drush-task
but generally if you use something similar to and any version of DrushTask.php it should work the same way!

build.xml for coder2.0:
https://github.com/shuairan/phing-drupal-template/compare/reload:master....

Hi douggreen,

I didn't tested it since, but I think drush should not throw an error return code when its actually warnings. It makes the build fail while it is perfectly ok to degrade temporarily the quality, its not to fail on this.

I think Shuairan explained more in depth why this is not needed than me, so I wont repeat him :)

I'm attaching my proposed fix for this issue. I just added an additional option (--ignores-pass) specifying that ignores will not count towards generating a failing shell exit code.

I'm using a pre-commit hook for a module I'm building that uses drush and coder review to ensure Drupal coding standards are in place, but unfortunately it involves CTools so I'm having to put some ignores here and there... this patch has worked very well for me.

-

I've wrongly tested patch, still needs review.

My Issue

I am in a very similar situation to what some of the previous posters are in: I'm running Jenkins tests against all new builds and the "Drush command terminated abnormally due to an unrecoverable error" errors are flagging all my builds as "Failed", regardless of whether or not Coder is properly being told to ingore the errors. The journey of finding out how to properly ignore only to learn that the error still gets thrown, led me to this thread and the patch from Comment #: 13. I took the following steps to test the patch and found that it appears to work for my needs:

My Testing

- Run the following command against a custom module file that extends a class from Views that uses snake-cased titles:

drush coder --no-empty --minor --comment --i18n --security --sql --style --druplart --sniffer --ignorename [my module]

- Confirmed that Drush was "terminated abnormally" in spite of the 3 confirmed ignores I had set in my _.module_ file.
- `cd`ed into `~/.drush` and renamed the `coder` folder to `coder-non-git`.
- I then cloned the Coder project into my `~/.drush` folder and applied the patch from Comment #: 13.
- Finally, I `cd`ed back to my project folder and ran the following:

drush coder --no-empty --minor --comment --i18n --security --sql --style --druplart --sniffer --ignorename --ignores-pass [my module]

- With the patch applied and the extra switch, I can confirm that Coder does not return an error and Drush does not throw the "Drush command terminated abnormally due to an unrecoverable error" message.

TLDR

Patch #: 13 worked for me.

I figured out how to apply patch #13 at build time in Jenkins, if anyone is curious. You can use Drush to download the Coder git repo & check out the latest stable branch using the following commands:

# Download Coder 7.x-2.x
drush dl coder --package-handler=git_drupalorg
# Use wget patch to enable using the `--ignores-pass` switch with Coder
wget -O sites/all/modules/coder/drush-ignores-pass.patch http://www.drupal.org/files/issues/1974654-drush-coder-review-summary-error-13.patch --no-check-certificate  
# Go to the coder directory so we can apply the patch
cd sites/all/modules/coder  
# Apply the patch to coder
git apply drush-ignores-pass.patch

Hopefully that might help somebody.

Committed, thanks!

Note that you can also make use of Coder Sniffer for any build integration which has a lot more checks and less false positives, see https://www.drupal.org/node/1419988