Page MenuHomePhabricator
Create Task
Maniphest T319202

ApiPrefixUniquenessTest::testPrefixes fails because linterstats and languagestats are both using "ls"
Closed, ResolvedPublic
Actions

Description

https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-noselenium-docker/57590/console

16:00:17 1) ApiPrefixUniquenessTest::testPrefixes
16:00:17 Module prefix 'ls' is shared between MediaWiki\Extension\Translate\Statistics\QueryLanguageStatsActionApi and MediaWiki\Linter\ApiQueryLinterStats
16:00:17 
16:00:17 /workspace/src/tests/phpunit/structure/ApiPrefixUniquenessTest.php:29
16:00:17 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:516
16:00:17 
16:00:17 FAILURES!
16:00:17 Tests: 9994, Assertions: 37561, Failures: 1, Skipped: 77.

Not clear why this is suddenly a problem.

Details

SubjectRepoBranchLines +/-
Mark some parserTests on talk pages Parsoid only on REL1_40mediawiki/coreREL1_40+10 -5
Mark some parserTests on talk pages Parsoid only on REL1_39mediawiki/coreREL1_39+10 -5
Zuul: [mediawiki/extensions/VisualEditor] Re-add DiscussionTools dependencyintegration/configmaster+2 -1
tests: suppress API prefix uniqueness check for 'ls'mediawiki/coremaster+2 -1
Revert "Add DiscussionTools as a dependency for VE"integration/configmaster+1 -2
ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflictmediawiki/extensions/LinterREL1_35+1 -1
ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflictmediawiki/extensions/LinterREL1_37+1 -1
ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflictmediawiki/extensions/LinterREL1_38+1 -1
ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflictmediawiki/extensions/LinterREL1_39+1 -1
ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflictmediawiki/extensions/Lintermaster+1 -1
Customize query in gerrit

Event Timeline

Nikerabbit created this task.Oct 3 2022, 2:08 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 3 2022, 2:08 PM
Nikerabbit triaged this task as Unbreak Now! priority.Oct 3 2022, 2:10 PM
Nikerabbit added projects: MediaWiki-extensions-Translate, MediaWiki-extensions-Linter.
Nikerabbit added a comment.Oct 3 2022, 2:20 PM

Seems to be https://gerrit.wikimedia.org/r/c/integration/config/+/833061: It adds DiscussionTools as a dependency to VisualEditor. Translate depends on VisualEditor and DiscussionTools depend on Linter.

gerritbot added a comment.Oct 3 2022, 2:25 PM

Change 837004 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] Revert "Add DiscussionTools as a dependency for VE"

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

gerritbot added a project: Patch-For-Review.Oct 3 2022, 2:25 PM
Jdforrester-WMF added a project: Continuous-Integration-Config.Oct 3 2022, 2:26 PM
gerritbot added a comment.Oct 3 2022, 2:47 PM

Change 837683 had a related patch set uploaded (by MSantos; author: MSantos):

[mediawiki/core@master] tests: suppress API prefix uniqueness check for 'ls'

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

gerritbot added a comment.Oct 3 2022, 3:05 PM

Change 837004 merged by jenkins-bot:

[integration/config@master] Revert "Add DiscussionTools as a dependency for VE"

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

MSantos added subscribers: xSavitar, daniel, Jdforrester-WMF, MSantos.Oct 3 2022, 3:11 PM

@Jdforrester-WMF a side note, we need DiscussionTools tests to run with VE to avoid other possible breakages. Not having those is a "soft" block on VE tests cc/ @daniel @xSavitar

Jdforrester-WMF added a comment.Oct 3 2022, 3:15 PM
In T319202#8280027, @MSantos wrote:

@Jdforrester-WMF a side note, we need DiscussionTools tests to run with VE to avoid other possible breakages. Not having those is a "soft" block on VE tests cc/ @daniel @xSavitar

Then it sounds like you have a blocker of getting your team's code in the Linter extension to be compatible with the Translate extension.

gerritbot added a comment.Oct 3 2022, 3:59 PM

Change 837693 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/Linter@master] ApiQueryLinterStats: Change the 'ls' prefix to 'linters' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:16 PM

Change 837693 merged by jenkins-bot:

[mediawiki/extensions/Linter@master] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:18 PM

Change 837706 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/Linter@REL1_39] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:18 PM

Change 837707 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/Linter@REL1_38] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:18 PM

Change 837708 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/Linter@REL1_37] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:18 PM

Change 837709 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/Linter@REL1_35] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

Jdforrester-WMF added a comment.Oct 3 2022, 4:20 PM

I've cherry-picked this to all release branches, but not to the deployment branches. If Translate needs a back-port to wmf.3 in the next couple of days, we'll need to also cherry-pick and deploy that change there too.

gerritbot added a comment.Oct 3 2022, 4:20 PM

Change 837706 merged by jenkins-bot:

[mediawiki/extensions/Linter@REL1_39] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:21 PM

Change 837707 merged by jenkins-bot:

[mediawiki/extensions/Linter@REL1_38] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:22 PM

Change 837708 merged by jenkins-bot:

[mediawiki/extensions/Linter@REL1_37] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

gerritbot added a comment.Oct 3 2022, 4:22 PM

Change 837709 merged by jenkins-bot:

[mediawiki/extensions/Linter@REL1_35] ApiQueryLinterStats: Change the 'ls' prefix to 'lntrst' to avoid conflict

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

Legoktm renamed this task from ApiPrefixUniquenessTest::testPrefixes to ApiPrefixUniquenessTest::testPrefixes fails because linterstats and languagestats are both using "ls".Oct 3 2022, 4:23 PM
ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.4; 2022-10-03).Oct 3 2022, 5:00 PM
gerritbot added a comment.Oct 3 2022, 5:49 PM

Change 837710 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] Zuul: [mediawiki/extensions/VisualEditor] Add DiscussionTools dependency [re-try]

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

Nikerabbit lowered the priority of this task from Unbreak Now! to High.Oct 4 2022, 7:10 AM

Thanks to everyone for the help!

gerritbot added a comment.Oct 5 2022, 1:18 PM

Change 837683 abandoned by Nikerabbit:

[mediawiki/core@master] tests: suppress API prefix uniqueness check for 'ls'

Reason:

Handled by changing prefix in the Linter extension

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

matmarex subscribed.Nov 14 2022, 11:43 PM

What remains to be done here?

Jdforrester-WMF added a comment.Nov 15 2022, 5:19 PM
In T319202#8394531, @matmarex wrote:

What remains to be done here?

I guess we left this open in case anyone was actually using the Linter stats via a bot and wanted a venue to complain. Definitely fixed now. Do we want to re-add the cyclical dependency as in https://gerrit.wikimedia.org/r/c/integration/config/+/837710 ?

matmarex added a comment.Nov 15 2022, 5:54 PM
In T319202#8396831, @Jdforrester-WMF wrote:

Do we want to re-add the cyclical dependency as in https://gerrit.wikimedia.org/r/c/integration/config/+/837710 ?

Does that actually do anything? VisualEditor tests don't need DiscussionTools to be installed to run.

daniel added a comment.Nov 15 2022, 7:59 PM
In T319202#8396931, @matmarex wrote:

Does that actually do anything? VisualEditor tests don't need DiscussionTools to be installed to run.

Changes to VE can break DT, so DT tests should run for every patch against VE.

matmarex added a comment.Nov 15 2022, 8:48 PM

Oh, it wasn't clear to me that this would do that. I thought that adding it to dependencies would just install it, but not also run its tests. But it turns out it does (the jobs just run the whole test suite, not the tests from a specific extension).

daniel added a comment.Nov 20 2022, 8:40 PM
In T319202#8397731, @matmarex wrote:

Oh, it wasn't clear to me that this would do that. I thought that adding it to dependencies would just install it, but not also run its tests. But it turns out it does (the jobs just run the whole test suite, not the tests from a specific extension).

I wish we could specify which kind of dependency it is - "needs to run" or "may break". We could skip the tests of stuff that is just needed to run.

matmarex closed this task as Resolved.Nov 30 2022, 9:48 PM

It doesn't seem like anyone wants to do anything else about this.

gerritbot added a comment.Feb 1 2024, 3:27 PM

Change 837710 merged by jenkins-bot:

[integration/config@master] Zuul: [mediawiki/extensions/VisualEditor] Re-add DiscussionTools dependency

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

Stashbot added a comment.Feb 1 2024, 3:29 PM

Mentioned in SAL (#wikimedia-releng) [2024-02-01T15:29:32Z] <James_F> Zuul: [mediawiki/extensions/VisualEditor] Re-add DiscussionTools dependency CC T319202

gerritbot added a comment.Feb 12 2024, 8:42 PM

Change 1002622 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/core@REL1_39] Mark some parserTests on talk pages Parsoid only on REL1_39

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

gerritbot added a comment.Feb 12 2024, 9:19 PM

Change 1002622 merged by jenkins-bot:

[mediawiki/core@REL1_39] Mark some parserTests on talk pages Parsoid only on REL1_39

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

Maintenance_bot removed a project: Patch-For-Review.Feb 12 2024, 9:32 PM
ReleaseTaggerBot added a project: MW-1.39-notes.Feb 12 2024, 10:00 PM
gerritbot added a comment.Wed, Mar 27, 7:16 PM

Change #1015099 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/core@REL1_40] Mark some parserTests on talk pages Parsoid only on REL1_40

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

gerritbot added a project: Patch-For-Review.Wed, Mar 27, 7:16 PM
gerritbot added a comment.Wed, Mar 27, 8:17 PM

Change #1015099 merged by jenkins-bot:

[mediawiki/core@REL1_40] Mark some parserTests on talk pages Parsoid only on REL1_40

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

ReleaseTaggerBot edited projects, added MW-1.40-notes; removed MW-1.40-notes (1.40.0-wmf.4; 2022-10-03).Wed, Mar 27, 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