Page MenuHomePhabricator
Create Task
Maniphest T206476

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

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.

Details

SubjectRepoBranchLines +/-
Call ob_start() before running testsmediawiki/coreREL1_31+4 -0
Call ob_start() before running testsmediawiki/coremaster+4 -0
Output only to stderr in unit testsmediawiki/coreREL1_31+3 -11
Output only to stderr in unit testsmediawiki/coremaster+5 -11
Customize query in gerrit

Related Objects

Event Timeline

Simetrical created this task.Oct 8 2018, 4:59 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 8 2018, 4:59 PM
Framawiki added a project: MediaWiki-Core-Tests.Oct 8 2018, 5:01 PM
Simetrical added a comment.Oct 8 2018, 5:02 PM

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

Simetrical added a comment.Oct 8 2018, 5:08 PM

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

Simetrical added a comment.Oct 8 2018, 5:10 PM

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

Simetrical added a comment.Oct 8 2018, 5:50 PM

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.

gerritbot subscribed.Oct 8 2018, 6:06 PM

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

gerritbot added a project: Patch-For-Review.Oct 8 2018, 6:06 PM
Legoktm added a project: PHP 7.2 support.Oct 9 2018, 6:12 AM
Krinkle moved this task from Untriaged to MediaWiki core on the PHP 7.2 support board.Oct 12 2018, 6:47 PM
Legoktm assigned this task to Simetrical.Oct 13 2018, 6:15 AM
gerritbot added a comment.Oct 13 2018, 6:29 AM

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
Legoktm merged a task: T182379: Test failures from PHPSessionHandlerTest in PHP 7.2.Oct 13 2018, 6:43 AM
Legoktm added subscribers: Reedy, Anomie, Tgr and 3 others.
ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-10-16 (1.32.0-wmf.26)).Oct 13 2018, 7:00 AM
Krinkle merged tasks: T196792: MediaWiki\Session\PHPSessionHandler::open: Wrong instance called, T197030: Warning: ini_set(): Headers already sent. You cannot change the session module's ini settings at this time in /srv/mediawiki/w/includes/session/PHPSessionHandler.php on line 126.Oct 14 2018, 10:52 PM
Krinkle awarded a token.
Krinkle moved this task from Inbox to PHPUnit on the MediaWiki-Core-Tests board.
gerritbot added a comment.Oct 16 2018, 12:47 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

gerritbot added a comment.Oct 16 2018, 1:16 AM

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

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

ReleaseTaggerBot added a project: MW-1.31-release-notes.Oct 16 2018, 2:00 AM
tstarling subscribed.May 25 2020, 3:56 AM

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.

gerritbot added a comment.May 25 2020, 3:57 AM

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

gerritbot added a comment.May 29 2020, 2:56 AM

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

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.35; 2020-06-02).May 29 2020, 3:00 AM
Maintenance_bot removed a project: Patch-For-Review.May 29 2020, 3:10 AM
gerritbot added a comment.Jun 1 2020, 7:29 PM

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

gerritbot added a project: Patch-For-Review.Jun 1 2020, 7:29 PM
gerritbot added a comment.Jun 1 2020, 7:58 PM

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

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

Maintenance_bot removed a project: Patch-For-Review.Jun 1 2020, 8:10 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:38 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits