Page MenuHomePhabricator

phpunit failure: Module prefix '' is shared between ApiQueryProofread and ApiTranscodeStatus
Open, NormalPublic

Description

Presumably visible only when run with ProofreadPage:

  1. PrefixUniquenessTest::testPrefixes

Module prefix '' is shared between ApiQueryProofread and ApiTranscodeStatus

/srv/vagrant/mediawiki/tests/phpunit/includes/api/PrefixUniquenessTest.php:21
/srv/vagrant/mediawiki/tests/phpunit/MediaWikiTestCase.php:133
/srv/vagrant/mediawiki/tests/phpunit/MediaWikiPHPUnitCommand.php:42
/srv/vagrant/mediawiki/tests/phpunit/phpunit.php:160


Version: unspecified
Severity: normal

Details

Reference
bz68648

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:39 AM
bzimport added a project: TimedMediaHandler.
bzimport set Reference to bz68648.
bzimport added a subscriber: Unknown Object (MLST).
Gilles triaged this task as Normal priority.Nov 24 2014, 2:31 PM
Gilles added a subscriber: Gilles.
Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 6:35 PM
Restricted Application added subscribers: Matanya, Aklapper. · View Herald TranscriptSep 4 2015, 6:35 PM
TheDJ added a subscriber: TheDJ.May 20 2017, 6:18 PM

These modules are subclasses of ApiQueryBase and apparently for these you are required to override the constructor in order to set the modify (but there is nothing that enforces anyone to do so etc..

TheDJ added a subscriber: Anomie.May 20 2017, 6:19 PM

@Anomie Did I conclude that correctly ? Is there something we can do to make this less of a pitfall ?

@Anomie Did I conclude that correctly ?

Yes, it seems so.

Is there something we can do to make this less of a pitfall ?

I suppose we could have ApiQueryBase::__construct() throw if the prefix is '', but that wouldn't stop two different extensions' modules from choosing the same prefix and only discovering that in a situation like this.

Or we could drop the prefix uniqueness test in favor of a test that ApiQueryBase modules all have distinct parameter names. Collisions there would be less likely (different prefixes mostly take care of it), but it would still be possible.

brion added a subscriber: brion.May 20 2017, 10:42 PM

We can probably change ApiTranscodeStatus to have a prefix... or kill it entirely in favor of videoinfo/imageinfo. It doesn't appear to actually be used within TMH, and it's not a great interface if anybody's using it.

yeah we used it in the transcode table before. At the moment we seem to just look in the database using queries to achieve the same. Kinda strange :)