Undefined property ContainerAwareEventDispatcherTest::results in run-tests.sh

This is due to run-tests.sh executing ContainerAwareEventDispatcherTest as a simpletest (web-test) instead of as a unit-test.

Note that the GUI test runner has been updated to support native PHPUnit_Framework_TestCase in #697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit), seems that test discovery of the run-test.sh script was forgotten then.

Note also that since #2189345: run-tests.sh should exit with a failure code if any tests failed, pifr now reports "Failed to run tests: failed during invocation of run-tests.sh." if anything fails in addition to the ContainerAwareEventDispatcherTest. Raising to critical, because this makes it rather painful to examine test failures on qa.drupal.org.

+1 to #4. Another example of where this happens: https://qa.drupal.org/pifr/test/910268.

+++ b/core/scripts/run-tests.sh
@@ -528,7 +528,7 @@ function simpletest_script_execute_batch($test_classes) {
+      if (is_subclass_of($test_class, 'PHPUnit_Framework_TestCase')) {

Put a \ in front of \PHPUnit_Framework_TestCase in both places, and you get an RTBC from me.

Delightful. :-)

Oh gosh this has been obscuring test results for every single one of my patches today :/ Thanks a lot folks!

Note that an existing issue was open since last may or so with an identical patch: #2225999: Drupal PHPUnit tests should extend UnitTestCase, not PHPUnit_Framework_TestCase and a lot of discussion.

Also, do we have an example somewhere that this change actually fixes the error reporting? Like a patch that includes this change and has fails...

I RTBC'd not because of error reporting (or bikeshedding... :-)) but because I want passing tests to pass the testbot.

Is the easiest way to accomplish that by changing run-tests.sh? Or by changing ContainerAwareEventDispatcherTest?

I can reproduce by passing the following to run-tests.sh --class 'Drupal\Tests\Component\EventDispatcher\ContainerAwareEventDispatcherTest'. Giving myself and @Wim Leers commit credit for the identical work in #2225999: Drupal PHPUnit tests should extend UnitTestCase, not PHPUnit_Framework_TestCase.

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed cbc449b and pushed to 8.0.x. Thanks!

Re #11: I assume the main reason for having to subclass PHPUnit_Framework_TestCase was static function getInfo(). This was only fixed in July by #697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit). The discussion in #2225999: Drupal PHPUnit tests should extend UnitTestCase, not PHPUnit_Framework_TestCase predates that. Also in the meantime #2270323: Remove DRUPAL_ROOT constant redefinitions from unit tests got in.

I also tested run-test.sh --list without and with the patch and the output is identical, so it does not look like test discovery is affected (i.e. PHPUnit_Framework_TestCase in vendor are not detected).

Re #12: Good call: #2320269-50: IGNORE: Patch testing issue (this is expected to throw one exception in ConfigImportAllTest).

Oh, #2320269-50: IGNORE: Patch testing issue ran through and the problem is not solved, it still says Failed to run tests: failed during invocation of run-tests.sh despite only one exception thrown and zero fails. Seems like pifr also needs to be updated for #2189345: run-tests.sh should exit with a failure code if any tests failed.

Yeah, I don't think this had anything to do with that problem but I wasn't sure so I asked in #12 :)

This error happened all the time, also in successful results, if it would have been related to the return status, then it would have always failed?

Happy that this fixed, was open for a long time, but not sure if the "Critical" was correct in retrospective and should be changed again? But I'm also OK it was critical for having notices in all test results and not properly running this test..

I had to revert this to revert #2189345: run-tests.sh should exit with a failure code if any tests failed. The approach in this patch is still valid and could go in before that one.

The patch in #2225999: Drupal PHPUnit tests should extend UnitTestCase, not PHPUnit_Framework_TestCase applies cleanly and the other issue has the proper name. Someone just could RTBC the other one :)

Update: Nevermind, that just got closed as a duplicate.

Reroll.

Alright, perfect!

Leaving this at critical because without this patch we are not running a test. This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 290712e and pushed to 8.0.x. Thanks!