Page MenuHomePhabricator

PasswordResetTest::testExecute_email errors out when CheckUser is installed
Closed, ResolvedPublic

Description

On a fresh installation of MediaWiki as of b347d8e where phpunit.php passed with no errors or failures, installing latest version of CheckUser as of 7a43255 and running the update script results in phpunit returning this error message:

1) PasswordResetTest::testExecute_email
Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? 
Query: INSERT  INTO `unittest_cu_changes` (cuc_namespace,cuc_title,cuc_minor,cuc_user,cuc_user_text,cuc_actiontext,cuc_comment,cuc_this_oldid,cuc_last_oldid,cuc_type,cuc_timestamp,cuc_ip,cuc_ip_hex,cuc_xff,cuc_xff_hex,cuc_agent) VALUES ('2','','0',NULL,NULL,'reset password for user \"User1\"','','0','0','3','20170918004929','1.2.3.4','01020304','0',NULL,'0')
Function: CheckUserHooks::updateCUPasswordResetData
Error: 1048 Column 'cuc_user' cannot be null (localhost)


/var/www/html/includes/libs/rdbms/database/Database.php:1149
/var/www/html/includes/libs/rdbms/database/Database.php:979
/var/www/html/includes/libs/rdbms/database/Database.php:1589
/var/www/html/extensions/CheckUser/CheckUser.hooks.php:124
/var/www/html/includes/Hooks.php:186
/var/www/html/includes/user/PasswordReset.php:203
/var/www/html/tests/phpunit/includes/user/PasswordResetTest.php:183
/var/www/html/maintenance/doMaintenance.php:92

This is because this line of code passes null for the second user, which works in MediaWiki core but fails when CheckUser is installed.

Details

Related Gerrit Patches:

Event Timeline

Huji created this task.Sep 18 2017, 1:10 AM

Change 378652 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/core@master] [WIP] Do not merge

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

Huji added a comment.Sep 18 2017, 1:28 AM

The issue with the above patch is that while it fixes the DB error, it results in the unit test not passing. The reason is that we do not pass the mock user to the PasswordReset::execute function; we pass the username, and on line 144 it tries to get the user from this username, but fails with a "nosuchuser" error.

I am not familiar enough with the unit testing feature to know how to fix this.

Huji added a subscriber: Tgr.EditedSep 18 2017, 7:08 PM
This comment has been deleted.
Tgr added a comment.Sep 18 2017, 7:17 PM

I imagine this was meant for the other bug? Here CheckUser chokes on the User::mailPasswordInternal hook, probably because those users don't actually exist in the database. I guess the test should either clear the relevant hooks or use real test users.

Change 378652 abandoned by Huji:
[WIP] Do not merge

Reason:
You are right. I found a way to fix it in CheckUser

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

Change 378772 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/extensions/CheckUser@master] Handle password reset in unit tests gracefully

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

Change 378652 restored by Huji:
[WIP] Do not merge

Reason:
Restoring per Grego's comment

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

Krinkle moved this task from Inbox to PHPUnit on the MediaWiki-Core-Testing board.Sep 19 2017, 6:09 PM

Change 378772 abandoned by Huji:
Handle password reset in unit tests gracefully

Reason:
Submitting a new patch on 378652 which would unregister the hook

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

Huji added a comment.Sep 25 2017, 4:51 PM

@Tgr now https://gerrit.wikimedia.org/r/#/c/378652/ is ready for final review. It does get the problem fixed.

Change 378652 merged by jenkins-bot:
[mediawiki/core@master] user: Unregister hooks that interfere with unit testing

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

Huji closed this task as Resolved.Sep 28 2017, 12:45 PM
Huji claimed this task.
Huji removed a project: Patch-For-Review.
Huji reopened this task as Open.Sep 28 2017, 3:12 PM
Huji added a subscriber: Krinkle.

@Krinkle okay, this patch doesn't get the job done. The unit test still ends with an error:

php tests/phpunit/phpunit.php tests/phpunit/includes/user/PasswordResetTest.php
Using PHP 7.0.22-0ubuntu0.16.04.1
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

...........E.

Time: 357 ms, Memory: 22.00MB

There was 1 error:

1) PasswordResetTest::testExecute_email
Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? 
Query: INSERT  INTO `unittest_cu_changes` (cuc_namespace,cuc_title,cuc_minor,cuc_user,cuc_user_text,cuc_actiontext,cuc_comment,cuc_this_oldid,cuc_last_oldid,cuc_type,cuc_timestamp,cuc_ip,cuc_ip_hex,cuc_xff,cuc_xff_hex,cuc_agent) VALUES ('2','','0',NULL,NULL,'reset password for user \"User1\"','','0','0','3','20170928151127','1.2.3.4','01020304','0',NULL,'0')
Function: CheckUserHooks::updateCUPasswordResetData
Error: 1048 Column 'cuc_user' cannot be null (localhost)


/var/www/html/includes/libs/rdbms/database/Database.php:1166
/var/www/html/includes/libs/rdbms/database/Database.php:979
/var/www/html/includes/libs/rdbms/database/Database.php:1607
/var/www/html/extensions/CheckUser/CheckUser.hooks.php:124
/var/www/html/includes/Hooks.php:177
/var/www/html/includes/Hooks.php:205
/var/www/html/includes/user/PasswordReset.php:203
/var/www/html/tests/phpunit/includes/user/PasswordResetTest.php:189
/var/www/html/tests/phpunit/MediaWikiTestCase.php:416
/var/www/html/maintenance/doMaintenance.php:92

Change 381231 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/core@master] Unregister hooks that interfere with unit testing

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

Change 381231 merged by jenkins-bot:
[mediawiki/core@master] Unregister hooks that interfere with unit testing

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

Huji closed this task as Resolved.Sep 28 2017, 5:10 PM
Huji removed a project: Patch-For-Review.
MarcoAurelio moved this task from Backlog to Closed on the CheckUser board.Nov 22 2017, 9:42 AM