DrupalStandardsListener documents that it only makes sense to receive TestCase objects as parameters to the listener. Unfortunately as is sometimes the case with phpUnit's internal, what makes sense isn't actually the case.
It actually gets weirder, when you have a @requires that isn't met, phpUnit passes a TestSuite into the listener. It doesn't make sense but we can just check the type and conditionally call our coverage test and move on. We'll miss checking the test that isn't run but I think that is ok.
| Issue category | Bug because it breaks phunit functionality for tests. |
|---|---|
| Issue priority | Normal bug |
| Unfrozen changes | Unfrozen because tests |
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2453627-1--drupal_standrads_listener_bug.patch | 1.27 KB | drunken monkey |
| Fix-DrupalStandardsListener-non-TestCase.patch | 1015 bytes | neclimdul |
Comments
Comment #1
drunken monkeyI think we can even do better and just recurse if we get a
TestSuiteobject. Then we'll only skip those tests if we get a class that isn't a subclass of either (if that can happen).Patch attached.
Also, adding #2415441: Automate finding @covers errors as a related issue to improve findability.
Comment #2
neclimdulYeah, that probably is better. It doesn't 100% solve my problem though because now those tests triggers #2443885: DrupalStandardsListener causes errors when it cannot retrieve test results. Missing abstract keyword.. :)
After that's in I think we can mark this RTBC.
Comment #3
drunken monkeyAh, yes, same for me …
But just when I wanted to comment there, it was committed. So if the patch works for you, please mark as RTBC (I don't want to do it for my own patch).
Comment #4
neclimdulAnd that's committed now so RTBC!
Comment #5
alexpottCommitted cdc9a44 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation to the issue summary