Page MenuHomePhabricator
Create Task
Maniphest T206296

PhpTags tests fail on PHP 7.1
Closed, ResolvedPublic
Actions

Description

Note: T188585: PhpTags tests fail on PHP 7.0

22:30:44 1) PhpTags\RuntimeTest::testRun_echo_assignment_29
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:264
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1271
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 2) PhpTags\RuntimeTest::testRun_echo_assignment_30
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:273
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1277
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 3) PhpTags\RuntimeTest::testRun_echo_assignment_31
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:282
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1283
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 4) PhpTags\RuntimeTest::testRun_echo_assignment_32
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1028
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1289
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 5) PhpTags\RuntimeTest::testRun_echo_assignment_33
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1038
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1295
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 6) PhpTags\RuntimeTest::testRun_echo_assignment_34
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1018
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1301
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 7) PhpTags\RuntimeTest::testRun_echo_assignment_35
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:300
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1307
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 8) PhpTags\RuntimeTest::testRun_echo_assignment_36
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:291
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1313
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 9) PhpTags\RuntimeTest::testRun_echo_assignment_37
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1058
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1319
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 10) PhpTags\RuntimeTest::testRun_echo_assignment_38
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1048
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:1325
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 11) PhpTags\RuntimeTest::testRun_echo_empty_array_push_3
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:958
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:2359
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94
22:30:44 
22:30:44 12) PhpTags\RuntimeTest::testRun_echo_empty_array_push_5
22:30:44 A non-numeric value encountered
22:30:44 
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:978
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:1226
22:30:44 /workspace/src/extensions/PhpTags/includes/Runtime.php:150
22:30:44 /workspace/src/extensions/PhpTags/tests/phpunit/includes/RuntimeTest.php:2371
22:30:44 /workspace/src/tests/phpunit/MediaWikiTestCase.php:416
22:30:44 /workspace/src/maintenance/doMaintenance.php:94

https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php71-docker/413/consoleFull

Details

SubjectRepoBranchLines +/-
PhpTags now passes php 7.1integration/configmaster+0 -23
Customize query in gerrit

Related Objects

Event Timeline

Legoktm created this task.Oct 5 2018, 5:47 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2018, 5:47 AM
Legoktm moved this task from Backlog to Other on the PHP 7.1 support board.Oct 5 2018, 6:10 AM
BPirkle claimed this task.Nov 14 2018, 1:03 AM
BPirkle subscribed.

Reproduced locally, investigating

BPirkle triaged this task as Medium priority.Nov 14 2018, 1:04 AM
BPirkle added a comment.Nov 14 2018, 3:15 AM

From http://php.net/manual/en/migration71.other-changes.php

New E_WARNING and E_NOTICE errors have been introduced when invalid strings are coerced using operators expecting numbers (+ - * / ** % << >> | & ^) or their assignment equivalents. An E_NOTICE is emitted when the string begins with a numeric value but contains trailing non-numeric characters, and an E_WARNING is emitted when the string does not contain a numeric value.

The failing tests all involve the affected operators. I will review to see what tests now make sense with this change in mind, and also what other impact this change may have to this extension.

BPirkle added a comment.Nov 15 2018, 5:51 PM

tl;dr: this extension (kind of) emulates PHP within a wiki page. How should it handle differences in PHP versions? Should it match the PHP version on the underlying web server, or define its own behavior? Either way has advantages and disadvantages.

The issue causing the failing phpunit tests is, as previously posted, a change in PHP behavior beginning with PHP 7.1.

When running under PHP 7.0, the following PhpTag code:

	<phptag>
	$foo = '7x';
	$bar = 5;
	echo $foo & $bar;
	</phptag>

Produces the text “5”. But when running under PHP 7.1+, that same PhpTag code (at least with my local settings) produces a broken submit page with this result:
“Notice: A non well formed numeric value encountered in /vagrant/mediawiki/extensions/PhpTags/includes/Runtime.php on line 273”

This is because of the PHP 7.1 change, which requires operands of the & operator to be numeric. Several other operators are similarly affected. A different wiki configuration might change the specifics of how the broken behavior appears, but none of them will be happy.

My first thought was to test the actual PHP version and if running on PHP 7.1+, require the inputs to be numeric. If they aren’t, display a PhpTags exception via the existing mechanism in the extension that allow cleanly display this category of error. This would correct the failing tests, the runtime error, mirror the behavior of actual PHP, and is reasonably easy to implement.

However, this would mean that the extension would behave differently depending on the version of PHP running on the server. It would make sharing PhpTags “code” between wikis potentially problematic - the same version of the extension would behave differently depending on the server it was running on. It would also mean that content could change in unexpected ways when a page was edited after a PHP upgrade on the underlying server. It is even theoretically possible that the same code could behave differently on different pages of the same wiki, or after an edit of a content page, if there were multiple underlying web servers running different versions of PHP. While that server configuration would be inadvisable, it is not impossible.

Alternatively, I could attempt to make the extension behave consistently across multiple PHP versions. However, this would mean the behavior of the extension sticks at whatever version we decide to track. This could also be confusing, because the Special:Version page would display one PHP version, but the PhpTags code behavior would not match that. It also means we’d have to continually emulate older language behavior in newer versions of PHP, which sounds difficult to implement and maintain.

My preference is to base the extension behavior on the actual PHP version. However, as an argument can be made either way, and as I’m not familiar with how this extension is used in practice, I thought it worth bringing up the point for discussion.

Who else should be included in this discussion?

BPirkle added subscribers: hashar, Pastakhov, Jdforrester-WMF.Nov 15 2018, 5:53 PM
BPirkle added a comment.Nov 19 2018, 3:56 AM

It is also worth noting that the test "testRun_echo_list_11" passes under PHP 7.0+. This test executes (via the PhpTags runtime engine) the following PhpTags code and checks the result:

list($bar[4], list($bar[], $bar[6][7][8])) = ["abcde", ["end", "-678678-"]];

In actual PHP 7.0+, this same code results in the following:

Notice: Undefined offset: 7 in /vagrant/mediawiki/local/T206296.php on line 34

That's because in PHP 7+, "list" performs variable assignments in left-to-right order, while in earlier versions of PHP is performs variable assignments in right-to-left order. The PhpTags extension implements the older behavior.

This is exactly the sort of situation I'm concerned about - if we don't update the extension to follow the behavior of newer PHP, then the extension is effectively emulating a version of PHP too old for newer versions of mediawiki to even run on. But if we do update the extension to follow the behavior of newer PHP, we may break existing wikis.

CCicalese_WMF added projects: Platform Engineering (PHP7 (TEC4)), Platform Team Workboards (Doing).Nov 20 2018, 3:29 AM
BPirkle removed BPirkle as the assignee of this task.Jan 25 2019, 2:41 PM
BPirkle edited projects, added Platform Team Legacy (Watching / External); removed Platform Team Workboards (Doing), Platform Engineering (PHP7 (TEC4)).
gerritbot added a comment.Aug 29 2019, 10:45 AM

Change 533187 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] PhpTags now passes php 7.1

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

gerritbot added a project: Patch-For-Review.Aug 29 2019, 10:45 AM
gerritbot added a comment.Aug 29 2019, 10:58 AM

Change 533187 merged by jenkins-bot:
[integration/config@master] PhpTags now passes php 7.1

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

Maintenance_bot removed a project: Patch-For-Review.Aug 29 2019, 11:11 AM
hashar closed this task as Resolved.Aug 29 2019, 11:50 AM

Somehow the tests are now passing on https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/PhpTags/+/441526/ :)

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