Page MenuHomePhabricator
Create Task
Maniphest T351848

TimeAdjustTest::testUserAdjust failure under php8.2 (strtotime handling of multiple signs changed)
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to replicate the issue (include links if applicable):

  • Run php8.2 tests

What happens?:

Using PHP 8.2.10
PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

[...]

There were 10 failures:

1) TimeAdjustTest::testUserAdjust with data set "Literal int -2 (technically undocumented)" ('20221015120000', -2, '20221015100000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015100000'
+'20221015140000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

2) TimeAdjustTest::testUserAdjust with data set "Literal -5" ('20221015120000', '-5', '20221015070000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015070000'
+'20221015170000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

3) TimeAdjustTest::testUserAdjust with data set "Literal -06:00" ('20221015120000', '-06:00', '20221015060000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015060000'
+'20221015180000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

4) TimeAdjustTest::testUserAdjust with data set "Full format -06:00" ('20221015120000', 'Offset|-360', '20221015060000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015060000'
+'20221015180000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

5) TimeAdjustTest::testUserAdjust with data set "Literal -06:45" ('20221015120000', '-06:45', '20221015051500')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015051500'
+'20221015184500'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

6) TimeAdjustTest::testUserAdjust with data set "Full format -06:45" ('20221015120000', 'Offset|-405', '20221015051500')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015051500'
+'20221015184500'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

7) TimeAdjustTest::testUserAdjust with data set "Literal -12:00" ('20221015120000', '-12:00', '20221015000000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015000000'
+'20221016000000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

8) TimeAdjustTest::testUserAdjust with data set "Full format -12:00" ('20221015120000', 'Offset|-720', '20221015000000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015000000'
+'20221016000000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

9) TimeAdjustTest::testUserAdjust with data set "Literal -13:00, capped at -12" ('20221015120000', '-13:00', '20221015000000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015000000'
+'20221016000000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

10) TimeAdjustTest::testUserAdjust with data set "Full format -13:00, capped at -12" ('20221015120000', 'Offset|-780', '20221015000000')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'20221015000000'
+'20221016000000'

/workspace/src/tests/phpunit/includes/language/TimeAdjustTest.php:16

What should have happened instead?:
Tests should passed, but negativ time correction is applied as positive correction.

Software version (skip for WMF-hosted wikis like Wikipedia): master (tests via https://gerrit.wikimedia.org/r/c/mediawiki/core/+/976316)

The test is calling Language::userAdjust, which calls DateTime::modify with "+{$minDiff} minutes" where $minDiff is the time correction to apply.
For negative correction this results in +-120 minutes, it seems the - is ignored in php8.2

DateTime::modify depends on strtotime to pass the modifier, but it seems that is broken:

# php -v
PHP 8.2.3 (cli) (built: Feb 14 2023 09:55:52) (ZTS Visual C++ 2019 x64)

# php maintenance\eval.php
> echo strtotime( "@1000000000 +120 minutes" );
1000007200
> echo strtotime( "@1000000000 -120 minutes" );
999992800
> echo strtotime( "@1000000000 +-120 minutes" );
1000007200
# php -v
PHP 8.1.8 (cli) (built: Jul  5 2022 23:04:29) (ZTS Visual C++ 2019 x64)

# php maintenance\eval.php
> echo strtotime( "@1000000000 +120 minutes" );
1000007200
> echo strtotime( "@1000000000 -120 minutes" );
999992800
> echo strtotime( "@1000000000 +-120 minutes" );
999992800

Details

SubjectRepoBranchLines +/-
language: Avoid multiple signs in Language::userAdjustmediawiki/coreREL1_40+1 -1
language: Avoid multiple signs in Language::userAdjustmediawiki/coreREL1_41+1 -1
language: Avoid multiple signs in Language::userAdjustmediawiki/coremaster+1 -1
Customize query in gerrit

Related Objects

Event Timeline

Umherirrender created this task.Nov 22 2023, 8:24 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 22 2023, 8:24 PM
Umherirrender added a comment.Nov 22 2023, 8:26 PM

https://github.com/php/php-src/issues/9950
https://github.com/php/doc-en/commit/ef51b1ce007349e7ff415ec5c707f1b5a73aba27

number symbols in relative formats no longer accept multiple signs, e.g. +-2

Not sure about impact about all the strtotime in the code and possible values stored in the database.

Umherirrender renamed this task from TimeAdjustTest::testUserAdjust failure under php8.2 (strtotime handling of +- changed) to TimeAdjustTest::testUserAdjust failure under php8.2 (strtotime handling of multiple signs changed).Nov 22 2023, 8:35 PM
gerritbot added a comment.Nov 22 2023, 8:42 PM

Change 976851 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@master] language: Avoid multiple signs in Language::userAdjust

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

gerritbot added a project: Patch-For-Review.Nov 22 2023, 8:42 PM
Umherirrender moved this task from Backlog to MediaWiki core on the PHP 8.2 support board.Nov 22 2023, 9:29 PM
gerritbot added a comment.Nov 24 2023, 11:41 AM

Change 976851 merged by jenkins-bot:

[mediawiki/core@master] language: Avoid multiple signs in Language::userAdjust

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

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.7; 2023-11-28).Nov 24 2023, 12:00 PM
Maintenance_bot removed a project: Patch-For-Review.Nov 24 2023, 12:10 PM
Jdforrester-WMF mentioned this in T352085: Make PHP 8.2 voting on development (master) branch of MW ecosystem (core, vendor, extensions, skins, libraries).Nov 27 2023, 7:36 PM
gerritbot added a comment.Nov 27 2023, 8:00 PM

Change 977622 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@REL1_41] language: Avoid multiple signs in Language::userAdjust

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

gerritbot added a project: Patch-For-Review.Nov 27 2023, 8:00 PM

Change 977623 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@REL1_40] language: Avoid multiple signs in Language::userAdjust

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

gerritbot added a comment.Nov 27 2023, 8:20 PM

Change 977622 merged by jenkins-bot:

[mediawiki/core@REL1_41] language: Avoid multiple signs in Language::userAdjust

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

gerritbot added a comment.Nov 27 2023, 8:22 PM

Change 977623 merged by jenkins-bot:

[mediawiki/core@REL1_40] language: Avoid multiple signs in Language::userAdjust

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

Umherirrender closed this task as Resolved.Nov 27 2023, 8:22 PM
Umherirrender claimed this task.
Umherirrender removed a project: Patch-For-Review.
ReleaseTaggerBot added projects: MW-1.41-notes, MW-1.40-notes.Nov 27 2023, 9:00 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