Page MenuHomePhabricator

Fix "PHP Warning: No route to fetch banner choice data configured" in QUnit runs
Closed, ResolvedPublic

Description

On a plain install with MediaWiki core (such as in the QUnit builds), the Jenkins build log show this PHP error:

PHP Warning: No route to fetch banner choice data configured. [Called from CNBannerChoiceDataResourceLoaderModule::getChoices in /mnt/jenkins-workspace/workspace/mwext-CentralNotice-qunit/src/extensions/CentralNotice/includes/CNBannerChoiceDataResourceLoaderModule.php at line 57]
in /mnt/jenkins-workspace/workspace/mwext-CentralNotice-qunit/src/includes/debug/MWDebug.php on line 300

This may be due to the modules we're testing having as a dependency the banner choice resource loader module.

In any case, it's not serious, only annoying, since it doesn't affect the ability of the job to detect actual QUnit failures or successes:
Fail: https://integration.wikimedia.org/ci/job/mwext-CentralNotice-qunit/490/console
Succeed: https://integration.wikimedia.org/ci/job/mwext-CentralNotice-qunit/526/console

Event Timeline

AndyRussG created this task.Mar 6 2015, 4:47 PM
AndyRussG raised the priority of this task from to Needs Triage.
AndyRussG updated the task description. (Show Details)
AndyRussG moved this task to Backlog on the MediaWiki-extensions-CentralNotice board.
AndyRussG added a subscriber: AndyRussG.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 6 2015, 4:47 PM
Krinkle renamed this task from Bug: fix meaningless error on Jenkins QUnit runs to Fix "PHP Warning: No route to fetch banner choice data configured" in QUnit runs.Mar 7 2015, 4:48 AM
Krinkle updated the task description. (Show Details)
Krinkle set Security to None.

Change 195200 had a related patch set uploaded (by AndyRussG):
QUnit: fix PHP error by setting $wgCentralDBname

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

Aklapper triaged this task as Normal priority.Mar 9 2015, 2:35 PM
awight added a subscriber: awight.Mar 14 2015, 9:27 PM

@AndyRussG: What do you think about deprecating the server banner choice feature? Would that help here?

@awight: That is something that I'd like to see go out ahead of the banner history et al. But it's not really much help here. The crux of this bug is that we should be mocking out the PHP-based bannerChoiceData module in QUnit tests of modules that have it as a dependency, and RL currently doesn't offer a way to do that! :)

How about mocking the API call, then? I did something like that for the BannerLoader call, you can temporarily take over $.ajax, or at a higher level...

The problem is in PHP. bannerController is loaded so it can be tested, and it has as a dependency bannerChoiceData, which is PHP-based. There's no way to tell the PHP code that this is a unit test (other than brittle hacks that would mix production and test code). CNBannerChoiceDataResourceLoaderModule (the PHP class that implements the bannerChoiceData module) runs normally, and throws a warning when it can't fetch choice data properly during the Jenkins job for the qunit test. Why exactly it can't fetch the choice data, I'm not sure. Something to do with the Jenkins configuration, or maybe the way Mediawiki is configured, or some default config, when it's run on Jenkins for the QUnit tests.

I have a very WIP patch for core that adds a "JS test mode" to ResourceLoader. It's not simple because of load.php la di da etc. etc., but it may be close to working.

An alternate workaround would be to fix the Jenkins job to properly run the API call. I think that'd also be OK. I imagine the unhackiness bar is not high at this point. :)

Krinkle added a subscriber: Krinkle.EditedMar 15 2015, 4:22 PM

I'd recommend principally against adding a generic/fragile tunnel that cuts through the infrastructure for testing. In my experience that tends to lead to situations where the application is not actually being tested well. It is also conceptually incompatible with how ResourceLoader neutralises requests into cacheable bundles. I don't think we can support or maintain this in a feasible manner. I know it wouldn't be used in production, but merely being there complicates maintenance. And the conceptual incompatibilities would likely lead to false positives or negatives in the test runs sooner or later.

Having said that, we actually already have various "flags" one could abuse for this purpose. Some documented, some not. For example, $wgEnableJavaScriptTest could be used to make the server behave differently. I'd recommend against that, but if you must, I'd try that instead.

I'm happy to schedule a quick IRC session to brainstorm towards a better solution. I don't know what the problem is at the moment so I'm unable to recommend what method to choose, but I'm certain we can find a suitable strategy using existing and proven infrastructure.

Change 196985 had a related patch set uploaded (by Awight):
Default to single-database configuration

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

Change 196986 had a related patch set uploaded (by Awight):
Do not include the bannerChoiceData RL module from tests

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

Change 196986 abandoned by Awight:
Do not include the bannerChoiceData RL module from tests

Reason:
This would be a disaster :)

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

Change 197003 had a related patch set uploaded (by Awight):
Use the one-argument form of wfGetDB when fetching the primary db

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

Change 197003 abandoned by Awight:
Clean up database switching

Reason:
squashed.

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

Change 197003 restored by Awight:
Clean up database switching

Reason:
ooops.

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

Change 196985 abandoned by Awight:
Default to single-database configuration

Reason:
squashed.

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

Change 197003 abandoned by Awight:
Clean up database switching

Reason:
aha! That was the one I meant to abandon.

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

Change 181238 had a related patch set uploaded (by Awight):
Clean up database switching

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

Change 196985 restored by Awight:
Default to single-database configuration

Reason:
Not squashed!

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

Change 181238 merged by jenkins-bot:
Clean up database switching

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

Change 197745 had a related patch set uploaded (by Awight):
Clean up database switching

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

atgo added a subscriber: atgo.

Adding to sprint as it's blocking a task in sprint

Change 196985 merged by jenkins-bot:
Default to single-database configuration

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

AndyRussG closed this task as Resolved.Mar 18 2015, 10:07 PM

Change 197745 abandoned by Awight:
Clean up database switching

Reason:
Already merged, as @I983cb3b5c882bb4884c76a94d4ebba39763fd299

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

mmodell removed a subscriber: awight.Jun 22 2017, 9:48 PM

Change 196882 abandoned by Krinkle:
WIP Add a JS test mode to RL

Reason:
Closing per the task. In a nut shell: For minor changes in source code behaviour, such as disallowing certain internal methods, one can use typeof QUnit.

For anything else, there are other methods in place for specific use cases.

If there's a use case you have and suspect might not be covered, as always, feel free to file at task :)

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