20:34:25 1) UnSubscribeUserTest::testUnSubscribeUser 20:34:25 MWException: CAS update failed on user_touched for user ID '3' (read from slave); the version of the user to be saved is older than the current version. 20:34:25 20:34:25 /mnt/jenkins-workspace/workspace/mwext-testextension-php55/src/includes/user/User.php:3984 20:34:25 /mnt/jenkins-workspace/workspace/mwext-testextension-php55/src/extensions/BounceHandler/includes/BounceHandlerActions.php:160 20:34:25 /mnt/jenkins-workspace/workspace/mwext-testextension-php55/src/extensions/BounceHandler/includes/BounceHandlerActions.php:86 20:34:25 /mnt/jenkins-workspace/workspace/mwext-testextension-php55/src/extensions/BounceHandler/includes/ProcessBounceEmails.php:91 20:34:25 /mnt/jenkins-workspace/workspace/mwext-testextension-php55/src/extensions/BounceHandler/tests/phpunit/UnSubscribeUserTest.php:58 20:34:25 /mnt/jenkins-workspace/workspace/mwext-testextension-php55/src/tests/phpunit/MediaWikiTestCase.php:389
Is BounceHandler really to blame here, or do we blame MW?
It's not like it's doing any of it's own DB queries, so it's MW being crappy
When I locally run it, I get this one:
There was 1 error: 1) UnSubscribeUserTest::testUnSubscribeUser Declaration of EchoBounceHandlerFormatter::getLinkParams() should be compatible with EchoBasicFormatter::getLinkParams($event, $user, $destination) /var/www/core/core/extensions/BounceHandler/includes/EchoBounceHandlerFormatter.php:3 /var/www/core/core/includes/AutoLoader.php:81 /var/www/core/core/extensions/Echo/includes/formatters/NotificationFormatter.php:101 /var/www/core/core/extensions/Echo/includes/controller/NotificationController.php:388 /var/www/core/core/extensions/Echo/includes/Notifier.php:101 /var/www/core/core/extensions/Echo/includes/controller/NotificationController.php:264 /var/www/core/core/extensions/Echo/includes/controller/NotificationController.php:104 /var/www/core/core/extensions/Echo/includes/model/Event.php:155 /var/www/core/core/extensions/BounceHandler/includes/BounceHandlerActions.php:107 /var/www/core/core/extensions/BounceHandler/includes/BounceHandlerActions.php:161 /var/www/core/core/extensions/BounceHandler/includes/BounceHandlerActions.php:86 /var/www/core/core/extensions/BounceHandler/includes/ProcessBounceEmails.php:91 /var/www/core/core/extensions/BounceHandler/tests/UnSubscribeUserTest.php:57 /var/www/core/core/tests/phpunit/MediaWikiTestCase.php:390
Well. it was due to my not-updated Echo. Now the error message is the same.
1) UnSubscribeUserTest::testUnSubscribeUser MWException: CAS update failed on user_touched for user ID '3' (read from slave); the version of the user to be saved is older than the current version.
Alright. So I took a look at the tests and the functions involved, and here are my findings.
- We are actually sending in 4 bounces and the limit is 3, now for the 4th one, the code at https://github.com/wikimedia/mediawiki-extensions-BounceHandler/blob/master/includes/BounceHandlerActions.php#L86 is actually trying to unsubscribe a user without checking if the user has their email account authenticated. The above patch does the additional checking.
- The tests are still failing, as I see that the user is getting unsubscribed in BounceHandlerActions::unSubscribeUser() and the authetnticationTimeStamp is still NULL. The funny thing is, when coming back to the test, trying to replicate this use again in https://github.com/wikimedia/mediawiki-extensions-BounceHandler/blob/master/tests/UnSubscribeUserTest.php -- I see that value of authenticationTimeStamp is set. Something wrong with the test framework, or what ?
Can someone take a look into #2, and help me get the code merged ?
I don't remember the details, beyond that User::save did not seem to clear the cache: loading the same user again resulted in the old data. Maybe it was some sort of in-process caching, or consistent reads resulting in pre-update DB data - I can't recall if I looked at those possibilities.
Is there any repro case of delete() not working? Seems more likely to be the use of pcTTL and READ_NORMAL (which is expected not to work). If you want to reload a User and see changes from 1ms ago, you need READ_LATEST. For unit test, clearing the WAN cache process cache via clearProcessCache() can also be an option if needed.