Page MenuHomePhabricator

Add test for conflict with popups
Closed, ResolvedPublic1 Estimated Story Points


Following T265872: Disable ReferencePreviews when gadget is detected there should be a test, that checks that there is only one popup at a time for reference previews.

Event Timeline

awight added a subscriber: awight.

Needs a story point estimate.

thiemowmde set the point value for this task to 3.Nov 18 2020, 1:54 PM

Adding a test turns out to be difficult. To test that the popup extension does not show a popup we have to somehow enable the relevant gadgets (and also the extension). We can not easily mock the gadgets, because the functions conflictsWithNavPopupsGadget() and conflictsWithRefTooltipsGadget() actually do check if there is a gadget installed. This would mean providing an even more complicated setup.
An alternative to mock the gadgets would be adding a cookie in the js test and reading it in conflictsWithRefTooltipsGadget() to set this variable to true in the test scenario. This would make the code kind of ugly and as this is not a beta feature it would have to stay permanently. But the alternative - installing the gadget on the CI during the test - would slow down the test and might be equally bad. We’re a bit torn here.

Lena_WMDE changed the point value for this task from 3 to 1.Dec 2 2020, 10:36 AM

One exotic alternative is to test in the browser test case setup that a conflicting gadget is installed. This positive test would be skipped in normal CI (we can still run the negative test "does not suppress when gadget disabled"). Then, we "permanently" enable a conflicting gadget on the beta cluster and run a daily job targeting beta. Both cases can also be run locally by toggling the gadget. This is dirty, but does give us automated coverage of both code paths.

We found a solution, that works with QUnit and tests the JavaScript part of the feature. It sets the variable on the JavaScript side to test the result - this way no installation of the gadget is needed. On the PHP side we already added the tests in the previous ticket, so both tests should cover the feature ok.

Change 644860 had a related patch set uploaded (by Svantje Lilienthal; owner: Svantje Lilienthal):
[mediawiki/extensions/Popups@master] Add browser test for conflict with popups

lilients_WMDE renamed this task from Add browser test for conflict with popups to Add test for conflict with popups.Dec 9 2020, 10:57 AM

Change 644860 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Add qunit test for conflict with popups