Page MenuHomePhabricator

SpecialCentralAuthTest fails when run in a suite with AccountCreationDetailsLookupTest
Closed, ResolvedPublic

Description

Problem
CentralAuth's SpecialCentralAuthTest class fails if run in a test suite with CheckUser's AccountCreationDetailsLookupTest before it. 1 test fails with Failed asserting that '<div...>' contains "(centralauth-admin-info-expired"..

Steps to reproduce
In a Mediawiki checkout with CheckUser, AntiSpoof, CentralAuth and GlobalBlocking enabled

  1. Copy phpunit.dist.xml to phpunit.xml
  2. Add a test suite with the following tests:
<testsuite name="failing_group">
  <file>extensions/CheckUser/tests/phpunit/integration/Services/AccountCreationDetailsLookupTest.php</file>
  <file>extensions/CentralAuth/tests/phpunit/integration/Special/SpecialCentralAuthTest.php</file>
</testsuite>
  1. Run the named test suite:
mw docker mediawiki exec -- MW_DB=wikidatawikidev composer run phpunit:entrypoint -- --testsuite failing_group

Observed behaviour
The test run fails:

> phpunit '--testsuite' 'failing_group'
Using PHP 8.1.18
Running with MediaWiki settings because there might be integration tests
PHPUnit 9.6.19 by Sebastian Bergmann and contributors.

............F..............                                       27 / 27 (100%)

Time: 00:06.516, Memory: 89.00 MB

There was 1 failure:

1) MediaWiki\Extension\CentralAuth\Tests\Phpunit\Integration\Special\SpecialCentralAuthTest::testViewForExistingGlobalTemporaryAccount
Failed asserting that '<div class='mw-htmlform-ooui-wrapper oo-ui-layout oo-ui-panelLayout oo-ui-panelLayout-padded oo-ui-panelLayout-framed'><form action='/wiki/Special:CentralAuth' method='get' enctype='application/x-www-form-urlencoded' class='mw-htmlform mw-htmlform-ooui oo-ui-layout oo-ui-formLayout'><fieldset class='oo-ui-layout oo-ui-labelElement oo-ui-fieldsetLayout'><legend class='oo-ui-fieldsetLayout-header'><span class='oo-ui-iconElement-icon oo-ui-iconElement-noIcon'></span><span class='oo-ui-labelElement-label'>(centralauth-admin-view)</span></legend><div class='oo-ui-fieldsetLayout-group'><div class='oo-ui-widget oo-ui-widget-enabled'><div data-mw-modules='ext.widgets.GlobalUserInputWidget' id='ooui-php-20' class='mw-htmlform-field-HTMLGlobalUserTextField mw-htmlform-autoinfuse oo-ui-layout oo-ui-labelElement oo-ui-fieldLayout oo-ui-fieldLayout-align-top' data-ooui='{"_":"mw.htmlform.FieldLayout","fieldWidget":{"tag":"target"},"align":"top","helpInline":true,"$overlay":true,"label":{"html":"(centralauth-admin-username)"},"classes":["mw-htmlform-field-HTMLGlobalUserTextField","mw-htmlform-autoinfuse"]}'><div class='oo-ui-fieldLayout-body'><span class='oo-ui-fieldLayout-header'><label for='ooui-php-19' class='oo-ui-labelElement-label'>(centralauth-admin-username)</label></span><div class='oo-ui-fieldLayout-field'><div id='target' class='oo-ui-widget oo-ui-widget-enabled oo-ui-inputWidget oo-ui-indicatorElement oo-ui-textInputWidget oo-ui-textInputWidget-type-text oo-ui-textInputWidget-php mw-widget-userInputWidget' data-ooui='{"_":"mw.widgets.GlobalUserInputWidget","$overlay":true,"name":"target","value":"~2024-1","inputId":"ooui-php-19","indicator":"required","required":true}'><input type='text' tabindex='0' name='target' value='~2024-1' required='' id='ooui-php-19' class='oo-ui-inputWidget-input' /><span class='oo-ui-iconElement-icon oo-ui-iconElement-noIcon'></span><span class='oo-ui-indicatorElement-indicator oo-ui-indicator-required'></span></div></div></div></div><div class="mw-htmlform-submit-buttons">\n
<span id='centralauth-submit-find' class='mw-htmlform-submit oo-ui-widget oo-ui-widget-enabled oo-ui-inputWidget oo-ui-buttonElement oo-ui-buttonElement-framed oo-ui-labelElement oo-ui-flaggedElement-primary oo-ui-flaggedElement-progressive oo-ui-buttonInputWidget' data-ooui='{"_":"OO.ui.ButtonInputWidget","type":"submit","value":"(centralauth-admin-lookup-ro)","label":"(centralauth-admin-lookup-ro)","flags":["primary","progressive"],"classes":["mw-htmlform-submit"]}'><button type='submit' tabindex='0' value='(centralauth-admin-lookup-ro)' class='oo-ui-inputWidget-input oo-ui-buttonElement-button'><span class='oo-ui-iconElement-icon oo-ui-iconElement-noIcon oo-ui-image-invert'></span><span class='oo-ui-labelElement-label'>(centralauth-admin-lookup-ro)</span><span class='oo-ui-indicatorElement-indicator oo-ui-indicatorElement-noIndicator oo-ui-image-invert'></span></button></span></div>\n
</div></div></fieldset></form></div><fieldset id="mw-centralauth-info">\n
<legend>(centralauth-admin-info-header)</legend>\n
<ul><li id="mw-centralauth-admin-info-username"><strong>(centralauth-admin-info-username)</strong> ~2024-1</li><li id="mw-centralauth-admin-info-registered"><strong>(centralauth-admin-info-registered)</strong> 08:33, 17 (september) 2024 ((centralauth-seconds-ago: −11,676,487))</li><li id="mw-centralauth-admin-info-editcount"><strong>(centralauth-admin-info-editcount)</strong> 0</li><li id="mw-centralauth-admin-info-attached"><strong>(centralauth-admin-info-attached)</strong> 1</li></ul>\n
</fieldset>\n
<fieldset>\n
<legend>(centralauth-admin-list-legend-ro)</legend>\n
<form method="post" action="/index.php?title=Special:CentralAuth/~2024-1&amp;action=submit" id="mw-centralauth-merged"><input type="hidden" value="unmerge" name="wpMethod"><input type="hidden" value="3953cc695db8a9813d7b5bc7ac04a07766371381+\" name="wpEditToken"><table class="wikitable sortable mw-centralauth-wikislist">\n
<thead><tr><th>(centralauth-admin-list-localwiki)</th><th>(centralauth-admin-list-attached-on)</th><th>(centralauth-admin-list-method)</th><th>(centralauth-admin-list-blocked)</th><th>(centralauth-admin-list-editcount)</th><th>(centralauth-admin-list-groups)</th></tr></thead><tbody><tr><td><a href="https://en.wikipedia.org/wiki/User:~2024-1" title="(centralauth-foreign-link: ~2024-1, en.wikipedia.org)">en.wikipedia.org</a></td><td data-sort-value="20240405060708">06:07, 5 (april) 2024</td><td style="text-align: center;"><img src="/extensions/CentralAuth/images/icons/merged-primary.png" alt="(centralauth-merge-method-primary)" title="(centralauth-merge-method-primary)"><span class="merge-method-help" title="(centralauth-merge-method-primary)" data-centralauth-mergemethod="primary">(centralauth-merge-method-questionmark)</span></td><td><a href="https://en.wikipedia.org/wiki/Special:Log/block?page=User:%7E2024-1" title="(centralauth-admin-blocklog)">(centralauth-admin-notblocked)</a></td><td style="text-align: right;"><a href="https://en.wikipedia.org/wiki/Special:Contributions/~2024-1" title="(centralauth-foreign-contributions: 0, en.wikipedia.org)">0</a></td><td></td></tr></tbody></table></form></fieldset>' contains "(centralauth-admin-info-expired".

/var/www/html/w/extensions/CentralAuth/tests/phpunit/integration/Special/SpecialCentralAuthTest.php:380

Expected Behaviour
The tests should pass

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change #1080261 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/CheckUser@master] Isolate temp account usage in testcase

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

Change #1080261 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Isolate temp account usage in testcase

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

abi_ triaged this task as Unbreak Now! priority.Oct 16 2024, 6:41 AM
abi_ subscribed.

Blocking us from merging patches in Translate repo. Example: 1073319: Special:ManageMessageGroupSubscriptions: Create page | https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Translate/+/1073319

Blocking us from merging patches in Translate repo. Example: 1073319: Special:ManageMessageGroupSubscriptions: Create page | https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Translate/+/1073319

I'll remove Translate from the list of extensions with parallel PHPUnit enabled.

Change #1080450 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] zuul: Don't run PHPUnit tests in parallel for Extension:Translate

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

Change #1080450 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] zuul: Don't run PHPUnit tests in parallel for Extension:Translate

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

I think this is a more global problem. See, for example, failing gate and submit jobs https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/1079626 failing with 'MediaWiki\Extension\CentralAuth\Tests\Phpunit\Integration\Special\SpecialCentralAuthTest::testViewForExistingGlobalTemporaryAccount'

I updated the patch to exclude Math as well. We checked ~20 repositories, so I think there are not too many more repositories with issues.

Change #1080459 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/CentralAuth@master] Tests: Skip testViewForExistingGlobalAccount

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

Change #1080459 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Tests: Skip testViewForExistingGlobalTemporaryAccount

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

Change #1080450 abandoned by Hashar:

[integration/config@master] zuul: Don't run PHPUnit tests in parallel for Translate and Math

Reason:

Abandoning in favor of skipping the faulty test in CentralAuth

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

kostajh lowered the priority of this task from Unbreak Now! to Needs Triage.Oct 16 2024, 9:07 AM

Change #1080702 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Kosta Harlan):

[mediawiki/extensions/CentralAuth@wmf/1.43.0-wmf.27] Tests: Skip testViewForExistingGlobalTemporaryAccount

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

Change #1080703 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Kosta Harlan):

[mediawiki/extensions/CentralAuth@wmf/1.43.0-wmf.26] Tests: Skip testViewForExistingGlobalTemporaryAccount

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

Change #1080450 restored by Kosta Harlan:

[integration/config@master] zuul: Don't run PHPUnit tests in parallel for Translate and Math

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

Change #1080702 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@wmf/1.43.0-wmf.27] Tests: Skip testViewForExistingGlobalTemporaryAccount

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

Change #1080703 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@wmf/1.43.0-wmf.26] Tests: Skip testViewForExistingGlobalTemporaryAccount

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

Mentioned in SAL (#wikimedia-operations) [2024-10-16T14:13:04Z] <lucaswerkmeister-wmde@deploy2002> Started scap sync-world: Backport for [[gerrit:1080702|Tests: Skip testViewForExistingGlobalTemporaryAccount (T377197)]], [[gerrit:1080669|Hard-code LabelCountField::NAME (T377226)]], [[gerrit:1080670|Remove LabelCountField (T377226)]], [[gerrit:1080671|Drop label_count field (LabelCountField) (T377226)]], [[gerrit:1080703|Tests: Skip testViewForExistingGlobalTemporaryAccount (T377197

Mentioned in SAL (#wikimedia-operations) [2024-10-16T14:15:18Z] <lucaswerkmeister-wmde@deploy2002> lucaswerkmeister-wmde: Backport for [[gerrit:1080702|Tests: Skip testViewForExistingGlobalTemporaryAccount (T377197)]], [[gerrit:1080669|Hard-code LabelCountField::NAME (T377226)]], [[gerrit:1080670|Remove LabelCountField (T377226)]], [[gerrit:1080671|Drop label_count field (LabelCountField) (T377226)]], [[gerrit:1080703|Tests: Skip testViewForExistingGlobalTemporaryAccount (T377197)]

Mentioned in SAL (#wikimedia-operations) [2024-10-16T14:24:41Z] <lucaswerkmeister-wmde@deploy2002> Finished scap sync-world: Backport for [[gerrit:1080702|Tests: Skip testViewForExistingGlobalTemporaryAccount (T377197)]], [[gerrit:1080669|Hard-code LabelCountField::NAME (T377226)]], [[gerrit:1080670|Remove LabelCountField (T377226)]], [[gerrit:1080671|Drop label_count field (LabelCountField) (T377226)]], [[gerrit:1080703|Tests: Skip testViewForExistingGlobalTemporaryAccount (T37719

Change #1081113 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/CheckUser@master] Scope every use of `enableAutoCreateTempUser` in test suite

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

Change #1081070 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/CentralAuth@master] Revert "Tests: Skip testViewForExistingGlobalTemporaryAccount"

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

Had another go at fixing the underlying issues. Should be ready for another attempt.

Change #1081113 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Scope every use of `enableAutoCreateTempUser` in test suite

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

Change #1081070 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Revert "Tests: Skip testViewForExistingGlobalTemporaryAccount"

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

I still see errors for math

21:33:33 1) MediaWiki\Extension\CentralAuth\Tests\Phpunit\Integration\Special\SpecialCentralAuthTest::testViewForExistingGlobalTemporaryAccount
21:33:33 Failed asserting that '<div class='mw-htmlform-ooui-wrapper oo-ui-layout oo-ui-panelLayout oo-ui-panelLayout-padded oo-ui-panelLayout-framed'><form action='/wiki/Special:CentralAuth' method='get' enctype='application/x-www-form-urlencoded' class='mw-htmlform mw-htmlform-ooui oo-ui-layout oo-ui-formLayout'><fieldset class='oo-ui-layout oo-ui-labelElement oo-ui-fieldsetLayout'><legend class='oo-ui-fieldsetLayout-header'><span class='oo-ui-iconElement-icon oo-ui-iconElement-noIcon'></span><span class='oo-ui-labelElement-label'>(centralauth-admin-view)</span></legend><div class='oo-ui-fieldsetLayout-group'><div class='oo-ui-widget oo-ui-widget-enabled'><div data-mw-modules='ext.widgets.GlobalUserInputWidget' id='ooui-php-114' class='mw-htmlform-field-HTMLGlobalUserTextField mw-htmlform-autoinfuse oo-ui-layout oo-ui-labelElement oo-ui-fieldLayout oo-ui-fieldLayout-align-top' data-ooui='{"_":"mw.htmlform.FieldLayout","fieldWidget":{"tag":"target"},"align":"top","helpInline":true,"$overlay":true,"label":{"html":"(centralauth-admin-username)"},"classes":["mw-htmlform-field-HTMLGlobalUserTextField","mw-htmlform-autoinfuse"]}'><div class='oo-ui-fieldLayout-body'><span class='oo-ui-fieldLayout-header'><label for='ooui-php-113' class='oo-ui-labelElement-label'>(centralauth-admin-username)</label></span><div class='oo-ui-fieldLayout-field'><div id='target' class='oo-ui-widget oo-ui-widget-enabled oo-ui-inputWidget oo-ui-indicatorElement oo-ui-textInputWidget oo-ui-textInputWidget-type-text oo-ui-textInputWidget-php mw-widget-userInputWidget' data-ooui='{"_":"mw.widgets.GlobalUserInputWidget","$overlay":true,"excludenamed":false,"excludetemp":false,"name":"target","value":"~2024-1","inputId":"ooui-php-113","indicator":"required","required":true}'><input type='text' tabindex='0' name='target' value='~2024-1' required='' id='ooui-php-113' class='oo-ui-inputWidget-input' /><span class='oo-ui-iconElement-icon oo-ui-iconElement-noIcon'></span><span class='oo-ui-indicatorElement-indicator oo-ui-indicator-required'></span></div></div></div></div><div class="mw-htmlform-submit-buttons">\n

Maybe revert the revert?

If it's sometimes, I was not lucky.

Change #1083808 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/CheckUser@master] Use `CheckUserTempUserTestTrait` to isolate temp users

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

Change #1083808 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Use `CheckUserTempUserTestTrait` to isolate temp users

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

Physikerwelt raised the priority of this task from High to Needs Triage.Oct 30 2024, 5:26 AM

Looks like I missed at least one use of TempUserTrait in CheckUser. I've uploaded a patch now which should resolve the issue in https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-noselenium/49533/console ( https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/1083452 / https://phabricator.wikimedia.org/T377167 )

Thank you. It works for me. From my perspective, the problem seems to be resolved.

This was seen on https://gerrit.wikimedia.org/r/c/integration/quibble/+/1084726, so I think we need to keep this task open.

What I don't get is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1081113 changed bunch of use TempUserTestTrait by use CheckUserTempUserTestTrait.

Next, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1083808 does another switch as a follow up.

Then the repository still has some usage of TempUserTestTrait and thus I imagine those might trigger the same issue again?

$ git grep -n 'use TempUserTestTrait'
tests/phpunit/integration/Api/ApiQueryCheckUserTest.php:42:     use TempUserTestTrait;
tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountLogHandlerTest.php:33:       use TempUserTestTrait;
tests/phpunit/integration/CheckUserTempUserTestTrait.php:11:    use TempUserTestTrait {
tests/phpunit/integration/GlobalContributions/SpecialGlobalContributionsTest.php:27:    use TempUserTestTrait;
tests/phpunit/integration/HookHandler/PerformRetroactiveAutoblockHandlerTest.php:22:    use TempUserTestTrait;
tests/phpunit/integration/IPContributions/SpecialIPContributionsTest.php:25:    use TempUserTestTrait;

Change #1084843 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/CheckUser@master] Use `CheckUserTempUserTestTrait` in more tests to isolate temp users

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

What I don't get is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1081113 changed bunch of use TempUserTestTrait by use CheckUserTempUserTestTrait.

Next, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1083808 does another switch as a follow up.

Then the repository still has some usage of TempUserTestTrait and thus I imagine those might trigger the same issue again?

$ git grep -n 'use TempUserTestTrait'
tests/phpunit/integration/Api/ApiQueryCheckUserTest.php:42:     use TempUserTestTrait;
tests/phpunit/integration/Api/Rest/Handler/TemporaryAccountLogHandlerTest.php:33:       use TempUserTestTrait;
tests/phpunit/integration/CheckUserTempUserTestTrait.php:11:    use TempUserTestTrait {
tests/phpunit/integration/GlobalContributions/SpecialGlobalContributionsTest.php:27:    use TempUserTestTrait;
tests/phpunit/integration/HookHandler/PerformRetroactiveAutoblockHandlerTest.php:22:    use TempUserTestTrait;
tests/phpunit/integration/IPContributions/SpecialIPContributionsTest.php:25:    use TempUserTestTrait;

I don't know why we didn't update those as well. Have done so now in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1084843

Change #1084843 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Use `CheckUserTempUserTestTrait` in more tests to isolate temp users

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

Change #1085390 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/CentralAuth@master] Revert^2 "Tests: Skip testViewForExistingGlobalTemporaryAccount"

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

Even after https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1084843, the CentralAuth test is still failing. I have proposed https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/1085390 to disable it again since that is breaking some other workflow such as cutting a new release of Quibble (which runs the full test suite).

I suggest that testViewForExistingGlobalTemporaryAccount to be harnessed to not rely on external effects such as CheckUser, then I don't know anything of the code. I imagine it is more complicated.

Change #1085390 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Revert^2 "Tests: Skip testViewForExistingGlobalTemporaryAccount"

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

Change #1085547 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/CentralAuth@master] Re-enable `testViewForExistingGlobalTemporaryAccount`

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

Change #1085547 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Re-enable `testViewForExistingGlobalTemporaryAccount`

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

ArthurTaylor claimed this task.

This has been re-enabled for a while now and we've had no new reports of issues. Closing this as resolved