Brings about memories of T189228 & T189329

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 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)?

Change 589306 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] entityLoaded: fire depending on relevant pages, not other modules

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

Change 589306 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] entityLoaded: fire depending on relevant pages, not other modules

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