Problem/Motivation

When stuff goes wrong with a PHPUnit test whilst it is being run by run-tests.sh. See https://www.drupal.org/pift-ci-job/251031 as an example.

Proposed resolution

Now more and more tests are PHPUnit based. Let's fix this.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#7 2707467-2.patch1002 bytesalexpott
#4 2707467-4.test-only.patch39.34 KBalexpott
#2 2707467-2.patch1002 bytesalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2707467-2.patch1002 bytes

Before

Drupal test run
---------------

Tests to be run:
  - \Drupal\Tests\system\Kernel\File\ExtensionStreamTest

Test run started:
  Saturday, April 16, 2016 - 03:13

Test summary
------------

\Drupal\Tests\system\Kernel\File\ExtensionStreamTest           0 passes   1 fails
FATAL \Drupal\Tests\system\Kernel\File\ExtensionStreamTest: test runner returned a non-zero error code (2).
\Drupal\Tests\system\Kernel\File\ExtensionStreamTest           0 passes   1 fails

Test run duration: 2 sec

Detailed test results
---------------------


---- \Drupal\Tests\system\Kernel\File\ExtensionStreamTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Fail      run-tests. Unknown              0 Unknown
    FATAL \Drupal\Tests\system\Kernel\File\ExtensionStreamTest: test runner
    returned a non-zero error code (2).

After

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\system\Kernel\File\ExtensionStreamTest

Test run started:
  Saturday, April 16, 2016 - 03:17

Test summary
------------

Drupal\Tests\system\Kernel\File\ExtensionStreamTest            0 passes   3 fails
FATAL Drupal\Tests\system\Kernel\File\ExtensionStreamTest: test runner returned a non-zero error code (2).
Drupal\Tests\system\Kernel\File\ExtensionStreamTest            0 passes   1 fails

Test run duration: 2 sec

Detailed test results
---------------------


---- Drupal\Tests\system\Kernel\File\ExtensionStreamTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Fail      Other      ExtensionStreamTe    0 Drupal\Tests\system\Kernel\File\Ext
    Drupal\Tests\system\Kernel\File\ExtensionStreamTest::testInvalidStreamUri
    with data set #0 ('invalid/uri')
    PHPUnit_Framework_Exception: PHP Fatal error:  Class
    'Symfony\Component\Filesystem\Filesystem' not found in
    /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Kernel/File/ExtensionStreamTest.php
    on line 79
    Fatal error: Class 'Symfony\Component\Filesystem\Filesystem' not found in
    /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Kernel/File/ExtensionStreamTest.php
    on line 79
Fail      Other      ExtensionStreamTe  188 Drupal\Tests\system\Kernel\File\Ext
    Drupal\Tests\system\Kernel\File\ExtensionStreamTest::testDirnameAsParameter
    PHPUnit_Framework_Exception: PHP Fatal error:  Class
    'Symfony\Component\Filesystem\Filesystem' not found in
    /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Kernel/File/ExtensionStreamTest.php
    on line 79
    Fatal error: Class 'Symfony\Component\Filesystem\Filesystem' not found in
    /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Kernel/File/ExtensionStreamTest.php
    on line 79
Fail      run-tests. Unknown              0 Unknown
    FATAL Drupal\Tests\system\Kernel\File\ExtensionStreamTest: test runner
    returned a non-zero error code (2).
dawehner’s picture

dawehner
Primary language German
commented

Does this end up on the testbot as well?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 2707467-4.test-only.patch39.34 KB

Combining with #1308152-281: Add stream wrappers to access .json files in extensions - the issue that inspired me to look at this. The test should fail and we should be able to see why.

Status: Needs review » Needs work

The last submitted patch, 4: 2707467-4.test-only.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review

Yay! DrupalCI is able to report proper errors from PHPUnit too. :) See https://www.drupal.org/pift-ci-job/251173 and compare with https://www.drupal.org/pift-ci-job/251031

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Task » Bug report
StatusFileSize
new 2707467-2.patch1002 bytes

Uploading #2 again since it is the patch to review and that should always be the last patch on the issue.

And I think it is bug that our test system does not report the correct errors from tests.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

It is quite nice that this didn't required more changes!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@dawehner I totally agree - I was very pleasantly surprised. 

+++ b/core/modules/simpletest/simpletest.module
@@ -181,6 +181,8 @@ function simpletest_run_tests($test_list) {
+  $rows = simpletest_phpunit_xml_to_rows($test_id, $phpunit_file);

If the xml file doesn't exist for some reason this won't cause a problem because of how it works... Here's the relevant snippet:

  $contents = @file_get_contents($phpunit_xml_file);
  if (!$contents) {
    return;
  }
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +rc target triage

I think this should be part of 8.1.0 as it will help people debug what is going wrong if write JavascriptTestBase tests and run them using run-tests.sh.

It also is 0 risk change for the public API of Drupal 8.

dawehner’s picture

dawehner
Primary language German
commented

I think this should be part of 8.1.0 as it will help people debug what is going wrong if write JavascriptTestBase tests and run them using run-tests.sh.

+1 This also doesn't cause a risk, as you proved above.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

I would love to see this get in. The number of "can you help me figure out why my test broke" requests would drop substantially.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Wow, this has bugged me for sooo long. I was even afraid to look at this issue because I was sure it involved some dark magic that went way over my head... @alexpott, you really are a master!!! RTBC+++++

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to both open 8.x. branches, thanks!

  • catch committed 5a4c730 on 8.1.x 
    Issue #2707467 by alexpott: Improve run-tests.sh phpunit error reporting...

  • catch committed f664dfa on 8.2.x 
    Issue #2707467 by alexpott: Improve run-tests.sh phpunit error reporting...
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -rc target triage +rc target
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +8.1.0 release notes

Status: Fixed » Closed (fixed)

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