18:23:41 15 04 2020 17:23:41.181:DEBUG [middleware:source-files]: Requesting /index.php/Special:EntityData/null.json?revision=0 18:23:41 15 04 2020 17:23:41.182:DEBUG [middleware:source-files]: Fetching /index.php/Special:EntityData/null.json 18:23:41 15 04 2020 17:23:41.182:DEBUG [proxy]: proxying request - /index.php/Special:EntityData/null.json?revision=0 to 127.0.0.1:9412 18:23:41 15 04 2020 17:23:41.669:DEBUG [HeadlessChrome 73.0.3683 (Linux 0.0.0)]: CONFIGURING -> EXECUTING 18:23:41 WARN: 'Pending requests does not match jQuery.active count' 18:23:41 testrunner 18:23:41 ✖️ Setup 18:23:41 WARN: 'Pending requests does not match jQuery.active count' 18:23:41 ✖️ Teardown 18:23:41 WARN: 'Pending requests does not match jQuery.active count' 18:23:41 ✖️ Loader status 18:23:41 WARN: 'Pending requests does not match jQuery.active count' 18:23:41 ✖️ assert.htmlEqual 18:23:41 WARN: 'Pending requests does not match jQuery.active count' 18:23:41 testrunner-after 18:23:41 ✖️ Teardown
|mediawiki/extensions/Wikibase||master||+15 -0||entityLoaded: fire depending on relevant pages, not other modules|
|Resolved||Michael||T250305 wmf-quibble-vendor-mysql-php72-docker failed on Special:EntityData|
|Resolved||• Pablo-WMDE||T251007 Remove entityViewInit's dependency on entityLoaded|
Interestingly, the suspicious requests against Special:EntityData/null.json?revision=0 are also happening on successful builds (e. g. the one subsequent to the randomly failing one that led to this ticket). It should be fixed but is possibly not at fault.
Looking at the combination of the null entity id and the specified revision I'm tempted to assume the request stems from where we try and fire the wikibase.entityPage.entityLoaded hook.
Suspicious requests against Special:EntityData/null.json?revision=0 happen in qunit tests when wikibase.entityPage.entityLoaded.js gets loaded and instantaneously run (even on no particular test, e. g.). RL Module Terminators Trailblazing may have had an effect on this. Guarding it against null entity ids could be a (naive) way forward.
Preventing this outgoing AJAX request may, like in the past, ensure that there are no pending AJAX requests upon afterEach. Chances are this only happens rarely because most of the time the response is quick enough.
That sounds reasonable to me. If I understand correctly, the wikibase.entityPage.entityLoaded module gets loaded because it’s a dependency of wikibase.ui.entityViewInit, which in turn is required by wikibase.view.tests (see view/tests/qunit/resources.php). Loading entityViewInit for the tests seems reasonable to me, and I don’t think we can/should make the entityLoaded module a conditional dependency, so the entityLoaded module probably has to deal with the possibility of being loaded in the unit tests, when there is no entity ID.
I wonder if it would make sense to check whether we’re in a unit test in entityLoaded? Something like this:
- If the entity ID is null:
- If we’re in a unit test:
- That’s fine, no need to worry about it. No-op.
- If we’re not in a unit test:
- There’s a bug, emit at least an mw.log.warn().
- If we’re in a unit test:
- If the entity ID is not null:
- Make the request. (Don’t bother checking if we’re in a unit test.)
I’m not sure what the best way to test “are we in a test” is – I found a check for window.QUnit being defined in a MediaWiki core file (resources/src/startup/mediawiki.js), but that’s the startup module, which is special in many ways. Maybe that kind of check is discouraged in other code?
If the entity ID is null: ...
I conceptually agree with the tree of options you drew but don't think we can responsibly implement the branch "If the entity ID is null > If we’re in a unit test" in production code (that's why i called the null check without a plan B naive).
Looking at wikibase.ui.entityViewInit, I wonder if wikibase.entityPage.entityLoaded can really be called its dependency - it's a script which triggers a hook. Making the firing of the hook itself a second class citizen (depend on something else) is kind of a double conditional. I think the hook should be a first class citizen (should fire when ever it is possible, i.e. FullEntityParserOutputGenerator) and who ever fancies should react to it (or not) how ever they please. Or in other words: GUI modules may rely on the hook to fire, but the important hook should not rely on some of our GUI module to have been used - and, from my point of view, RL is not right way to model that relationship.
What do you think (conceptually)?