When we fixed #2560643: Fatal PHPUnit tests are not always reported as failure, we introduced a regression where a legitimately failing phpunit test now returns less information that it did before.

http://cgit.drupalcode.org/drupal/tree/core/modules/simpletest/simpletes... is only checking if $ret is anything other than 0, which works for segfaulting or php fatal error issues (they return 139 and 255) but it is also catching normal test failures which returns 1.

If phpunit returns 1, we can safely assume that the xml file was generated and that we should use those results.

CommentFileSizeAuthor
#8 2582457-8.phpunit_fail_reporting.patch1.39 KBMixologic
#2 2582457-2.phpunit_fail_reporting.patch805 bytesMixologic
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mixologic created an issue. See original summary.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Issue tags: +Needs tests
FileSize
2582457-2.phpunit_fail_reporting.patch805 bytes
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Active » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/simpletest/simpletest.module
@@ -179,7 +179,9 @@ function simpletest_run_tests($test_list) {
-  if ($ret) {
+  // A return value of 0 = passed test, 1 = failed test, > 1 indicates segfault
+  // timeout, or other type of failure.
+  if ($ret > 1) {

Is there any way to test that? I guess we have simply no chance.

I think we still should run phpunit tests natively.

Here is some advertisment for another issue which would make that possible: #2567029: Be able to run only simpletest tests, but no phpunit, running from run-tests.sh

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented

Well, one of the ways Im testing it was to resubmit a different patch with this fix in it:

https://www.drupal.org/node/1425588#comment-10425655

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2582457-2.phpunit_fail_reporting.patch, failed testing.

The last submitted patch, 2: 2582457-2.phpunit_fail_reporting.patch, failed testing.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs work » Needs review
FileSize
2582457-8.phpunit_fail_reporting.patch1.39 KB

The test that tests this should test a fail > 1.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

I think that should count.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Ha, we sort of have a test, it was just wrong :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Let's see if this change would have caught #2592367: PHPUnit is broken - we have dependencies on Symfony event dispatcher tests.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

It seems to not have caught it?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue tags: +rc eligible

This is test only

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented

#2592367: PHPUnit is broken - we have dependencies on Symfony event dispatcher tests. was actually related to #2580293: Patch having test with "PHP Fatal error" is marked as PASSED. This would not have caught that.

The original issue #2560643: Fatal PHPUnit tests are not always reported as failure only managed to work for PHP Unit tests that Fatal as part of its runtime execution - If it had a compile time Fatal (like a syntax error, or class not found errors) then these fatals did the "disappearing act"

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed cc3ddd4 and pushed to 8.0.x. Thanks!

  • alexpott committed cc3ddd4 on 8.0.x 
    Issue #2582457 by Mixologic, dawehner: Failed PHP Unit tests should...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.