Problem/Motivation

If you do something like:

    $this->assertEquals('/a-url-that-does-not-match', $this->getSession()->getCurrentUrl());

This will populate BrowserTestBase::$mink and BrowserTestBase::$container, neither or which can be serialised. And the PHPUnit result will contain a stack trace which include the test object which will fail when serialized as part of the way tests in separate processes are run.

Steps to reproduce

  1. Add $this->assertSame('asd', 'a'); as the first line of BrowserTestBaseTest::testGoTo()
  2. Step up the environmental variables so that tests work
  3. phpunit -c core core/modules/simpletest/tests/src/Functional/BrowserTestBaseTest.php

Expected result

1) Drupal\Tests\simpletest\Functional\BrowserTestBaseTest::testGoTo
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-asd
+a

/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Constraint.php:110
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:88
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Assert.php:2183
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Assert.php:1174
/Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/simpletest/tests/src/Functional/BrowserTestBaseTest.php:30
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestCase.php:889
/Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/simpletest/src/BrowserTestBase.php:822
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestCase.php:749
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestResult.php:601
/Volumes/devdisk/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestCase.php:705

Actual result

1) Drupal\Tests\simpletest\Functional\BrowserTestBaseTest::testGoTo
PHPUnit_Framework_Exception: PHP Fatal error:  Uncaught exception 'LogicException' with message 'The database connection is not serializable. This probably means you are serializing an object that has an indirect reference to the database connection. Adjust your code so that is not necessary. Alternatively, look at DependencySerializationTrait as a temporary solution.' in /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Database/Connection.php:1433
Stack trace:
#0 [internal function]: Drupal\Core\Database\Connection->__sleep()
#1 -(70): serialize(Array)
#2 -(83): __phpunit_run_isolated_test()
#3 {main}
  thrown in /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Database/Connection.php on line 1433

The same problem afflicts KernelTestBase tests see #6

Eligible for 8.0.x because it is a test only change.

Proposed resolution

Don't serialise any properties - there is no point.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
FileSize
959 bytes
alexpott’s picture

Issue summary: View changes
FileSize
959 bytes
881 bytes
alexpott’s picture

Issue summary: View changes
dawehner’s picture

Interesting!

Don't serialise any properties - there is no point.

Totally agree with that.

Should we add it to KernelTestBase as well, people could easily do a similar mistake, like storing something in setup()

alexpott’s picture

Title: BrowserTestBase tests break badly when they fail » BrowserTestBase and KernelTestBase tests break badly when they fail
Issue summary: View changes
FileSize
1.83 KB
910 bytes

@dawehner yep - this can afflict KernelTestBase too... just do $this->db = $this->container->get('database'); and the assert in the issue summary and it breaks the same way.

dawehner’s picture

Status: Needs review » Needs work

Can we please have some tests?

alexpott’s picture

Status: Needs work » Needs review

I have no idea how to test this - the error occurs in PHPUnit code outside of the test method.

bojanz’s picture

Testing a test sounds a bit meta.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Fair. Its indeed pretty hard to test test failures.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

  • catch committed 38e1141 on 8.1.x
    Issue #2636228 by alexpott: BrowserTestBase and KernelTestBase tests...

  • catch committed 86c03d8 on
    Issue #2636228 by alexpott: BrowserTestBase and KernelTestBase tests...

Status: Fixed » Closed (fixed)

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