Page MenuHomePhabricator

PHPSessionHandlerTest::testSessionHandling fails on PHP 7.2 when run after PHPSessionHandlerTest::testInstall
Closed, ResolvedPublic

Description

aryeh@monoid:/var/www/html/w$ php7.0 tests/phpunit/phpunit.php tests/phpunit/includes/session/PHPSessionHandlerTest.php --filter 'testInstall|testSessionHandling'
Using PHP 7.0.32-1+ubuntu16.04.1+deb.sury.org+1
PHPUnit 6.5.13 by Sebastian Bergmann and contributors.

....                                                                4 / 4 (100%)

Time: 248 ms, Memory: 30.00MB

OK (4 tests, 65 assertions)
aryeh@monoid:/var/www/html/w$ php7.1 tests/phpunit/phpunit.php tests/phpunit/includes/session/PHPSessionHandlerTest.php --filter 'testInstall|testSessionHandling'
Using PHP 7.1.22-1+ubuntu16.04.1+deb.sury.org+1
PHPUnit 6.5.13 by Sebastian Bergmann and contributors.

....                                                                4 / 4 (100%)

Time: 408 ms, Memory: 30.00MB

OK (4 tests, 65 assertions)
aryeh@monoid:/var/www/html/w$ php7.2 tests/phpunit/phpunit.php tests/phpunit/includes/session/PHPSessionHandlerTest.php --filter 'testInstall|testSessionHandling'
Using PHP 7.2.10-1+ubuntu16.04.1+deb.sury.org+1
PHPUnit 6.5.13 by Sebastian Bergmann and contributors.

PHP Warning:  ini_set(): Headers already sent. You cannot change the session module's ini settings at this time in /var/www/html/w/tests/phpunit/includes/session/PHPSessionHandlerTest.php on line 79
PHP Stack trace:
PHP   1. {main}() /var/www/html/w/tests/phpunit/phpunit.php:0
PHP   2. require() /var/www/html/w/tests/phpunit/phpunit.php:129
PHP   3. PHPUnitMaintClass->execute() /var/www/html/w/maintenance/doMaintenance.php:94
PHP   4. MediaWikiPHPUnitCommand->run() /var/www/html/w/tests/phpunit/phpunit.php:90
PHP   5. MediaWikiTestRunner->doRun() /var/www/html/w/vendor/phpunit/phpunit/src/TextUI/Command.php:195
PHP   6. PHPUnit\Framework\TestSuite->run() /var/www/html/w/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:545
PHP   7. MediaWiki\Session\PHPSessionHandlerTest->run() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestSuite.php:755
PHP   8. MediaWiki\Session\PHPSessionHandlerTest->run() /var/www/html/w/tests/phpunit/MediaWikiTestCase.php:416
PHP   9. MediaWikiTestResult->run() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:894
PHP  10. MediaWiki\Session\PHPSessionHandlerTest->runBare() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestResult.php:698
PHP  11. MediaWiki\Session\PHPSessionHandlerTest->runTest() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:939
PHP  12. ReflectionMethod->invokeArgs() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:1071
PHP  13. MediaWiki\Session\PHPSessionHandlerTest->testInstall() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:1071
PHP  14. ini_set() /var/www/html/w/tests/phpunit/includes/session/PHPSessionHandlerTest.php:79
PHP Warning:  ini_set(): Headers already sent. You cannot change the session module's ini settings at this time in /var/www/html/w/tests/phpunit/includes/session/PHPSessionHandlerTest.php on line 80
PHP Stack trace:
PHP   1. {main}() /var/www/html/w/tests/phpunit/phpunit.php:0
PHP   2. require() /var/www/html/w/tests/phpunit/phpunit.php:129
PHP   3. PHPUnitMaintClass->execute() /var/www/html/w/maintenance/doMaintenance.php:94
PHP   4. MediaWikiPHPUnitCommand->run() /var/www/html/w/tests/phpunit/phpunit.php:90
PHP   5. MediaWikiTestRunner->doRun() /var/www/html/w/vendor/phpunit/phpunit/src/TextUI/Command.php:195
PHP   6. PHPUnit\Framework\TestSuite->run() /var/www/html/w/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:545
PHP   7. MediaWiki\Session\PHPSessionHandlerTest->run() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestSuite.php:755
PHP   8. MediaWiki\Session\PHPSessionHandlerTest->run() /var/www/html/w/tests/phpunit/MediaWikiTestCase.php:416
PHP   9. MediaWikiTestResult->run() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:894
PHP  10. MediaWiki\Session\PHPSessionHandlerTest->runBare() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestResult.php:698
PHP  11. MediaWiki\Session\PHPSessionHandlerTest->runTest() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:939
PHP  12. ReflectionMethod->invokeArgs() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:1071
PHP  13. MediaWiki\Session\PHPSessionHandlerTest->testInstall() /var/www/html/w/vendor/phpunit/phpunit/src/Framework/TestCase.php:1071
PHP  14. ini_set() /var/www/html/w/tests/phpunit/includes/session/PHPSessionHandlerTest.php:80
.SSE                                                                4 / 4 (100%)

Time: 249 ms, Memory: 32.25MB

There was 1 error:

1) MediaWiki\Session\PHPSessionHandlerTest::testSessionHandling with data set #2 ('php_serialize')
UnexpectedValueException: MediaWiki\Session\PHPSessionHandler::open: Wrong instance called!

/var/www/html/w/includes/session/PHPSessionHandler.php:180
/var/www/html/w/tests/phpunit/includes/session/PHPSessionHandlerTest.php:149
/var/www/html/w/tests/phpunit/MediaWikiTestCase.php:416
/var/www/html/w/maintenance/doMaintenance.php:94

ERRORS!
Tests: 4, Assertions: 8, Errors: 1, Skipped: 2.

This also causes a lot of tests afterwards to fail with "Trying to get property 'num_rows' of non-object" at DatabaseMysqli.php:233.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 8 2018, 4:59 PM

I'm trying to bisect now to see if this is a regression. If headers are already sent, that seems like a problem?

A revision from 2017 had the same problem, so it's safe to assume this isn't a regression in MediaWiki.

The "Headers already sent" is just because capitalization changed in PHP 7.2 and evaded our error suppression, so that's presumably not related.

Looks like it's this: https://bugs.php.net/bug.php?id=75628 Seems we have to make sure this test actually doesn't output anything.

Change 465201 had a related patch set uploaded (by simetrical; owner: simetrical):
[mediawiki/core@master] Output only to stderr in unit tests

https://gerrit.wikimedia.org/r/465201

Change 465201 merged by jenkins-bot:
[mediawiki/core@master] Output only to stderr in unit tests

https://gerrit.wikimedia.org/r/465201

Legoktm closed this task as Resolved.Oct 13 2018, 6:33 AM

Change 467549 had a related patch set uploaded (by Legoktm; owner: simetrical):
[mediawiki/core@REL1_31] Output only to stderr in unit tests

https://gerrit.wikimedia.org/r/467549

Change 467549 merged by jenkins-bot:
[mediawiki/core@REL1_31] Output only to stderr in unit tests

https://gerrit.wikimedia.org/r/467549

I came across a variation of this today at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/TEI/+/597681 . The problem is that an implicit policy of not sending output to stdout creates a non-obvious failure mode in which one test breaks another. Today's problem involves CI being configured with display_errors=1, causing headers to be sent when wfDeprecated() is called from a data provider.

Calling ob_start() sufficiently early fixes the problem. I'm documenting this here since I'll reference this task in my commit.

Change 598293 had a related patch set uploaded (by Tim Starling; owner: Tim Starling):
[mediawiki/core@master] Call ob_start() before running tests

https://gerrit.wikimedia.org/r/598293

Change 598293 merged by jenkins-bot:
[mediawiki/core@master] Call ob_start() before running tests

https://gerrit.wikimedia.org/r/598293

Change 601397 had a related patch set uploaded (by SBassett; owner: Tim Starling):
[mediawiki/core@REL1_31] Call ob_start() before running tests

https://gerrit.wikimedia.org/r/601397

Change 601397 merged by jenkins-bot:
[mediawiki/core@REL1_31] Call ob_start() before running tests

https://gerrit.wikimedia.org/r/601397