Page MenuHomePhabricator
Create Task
Maniphest T208808

[Spike, 2hrs] Identify the problem with the toast notification tests on the beta cluster
Closed, ResolvedPublic
Actions

Description

To date, the last three Minerva builds have had consecutive failures from tests using the Then I should see a toast notification step [1][2][3]

The tests are sometimes failing on the step "I should see a toast notification" which might be related to the fact that the toast displays asynchronously [4]. I cannot replicate on the production site which makes me think the issue is with the test itself. It also seems there was a previous attempt to address this in T170890, but it may not have fully resolved the issue.

Investigation needs to take place to uncover why this is still occurring and provide a solution to fix it.

[1] https://integration.wikimedia.org/ci/view/Reading-Web/job/selenium-MinervaNeue/BROWSER=firefox,MEDIAWIKI_ENVIRONMENT=beta,PLATFORM=Linux,label=BrowserTests/lastCompletedBuild/testReport/(root)/Page%20actions%20menu%20when%20anonymous/I_cannot_edit_a_protected_page_when_anonymous/
[2] https://integration.wikimedia.org/ci/view/Reading-Web/job/selenium-MinervaNeue/BROWSER=firefox,MEDIAWIKI_ENVIRONMENT=beta,PLATFORM=Linux,label=BrowserTests/lastCompletedBuild/testReport/(root)/Search/Clicking_on_a_watchstar_toggles_the_watchstar/
[3] https://integration.wikimedia.org/ci/view/Reading-Web/job/selenium-MinervaNeue/BROWSER=firefox,MEDIAWIKI_ENVIRONMENT=beta,PLATFORM=Linux,label=BrowserTests/lastCompletedBuild/testReport/(root)/Wikitext%20Editor%20(Makes%20actual%20saves)/It_is_possible_to_edit/
[4] https://phabricator.wikimedia.org/source/mediawiki/browse/master/resources/src/mediawiki.notify.js

Developer notes

Things look like they are working well, it's just a problem with the test.

886c37e94f6 worthy of investigation.

Outcome

  • This task is updated / a new bug is created documenting how to fix the problem.
  • If the change is trivial and you can find a +2er feel free to fix it.

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : masterQA: Drop protected page test
mediawiki/skins/MinervaNeue : masterDrop toast tests from beta cluster
mediawiki/skins/MinervaNeue : masterQA: Remove hacks introduced for T170890
mediawiki/skins/MinervaNeue : masterQA: Drop more toast notification tests from Firefox (logged in search)
mediawiki/skins/MinervaNeue : masterQA: Drop some tests in Firefox

Related Objects

Event Timeline

nray created this task.Nov 5 2018, 10:13 PM
Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptNov 5 2018, 10:13 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
nray updated the task description. (Show Details)Nov 5 2018, 10:16 PM
nray renamed this task from Fix "Page actions menu when anonymous.I cannot edit a protected page when anonymous" test to Fix toast notification tests.Nov 5 2018, 10:41 PM
nray updated the task description. (Show Details)
Jdlrobson added a subscriber: Jdlrobson.Nov 5 2018, 10:42 PM

It could be a host of things. When I view the saucelab videos I see the toast in the video, so it's definitely there. Just for some reason, Selenium is not able to detect it in the HTML.

Two possible theories:

  1. times out waiting for the toast to appear
  2. Problem with css selector div(:toast, css: '.mw-notification')
nray updated the task description. (Show Details)Nov 5 2018, 11:06 PM
Jdlrobson updated the task description. (Show Details)Nov 6 2018, 10:38 PM
Jdlrobson renamed this task from Fix toast notification tests to [Spike, 2hrs] Identify the problem with the toast notification tests on the beta cluster.Nov 9 2018, 9:11 PM
Jdlrobson added a project: Spike.
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Needs Prioritization to Upcoming on the Readers-Web-Backlog board.
Jdlrobson closed this task as Resolved.Nov 15 2018, 6:46 PM
Jdlrobson claimed this task.

Seems to have been fixed by https://gerrit.wikimedia.org/r/473313

gerritbot added a subscriber: gerritbot.Nov 15 2018, 8:53 PM

Change 473841 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] QA: Drop some tests in Firefox

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

gerritbot added a comment.Nov 15 2018, 9:03 PM

Change 473841 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] QA: Drop some tests in Firefox

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

gerritbot added a comment.Nov 15 2018, 10:33 PM

Change 473939 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] QA: Remove hacks introduced for T170890

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

Jdlrobson added a comment.Nov 16 2018, 12:10 AM

In all the failed tests the toast is shown, but when the toast is checked it has disappeared.
Some hacks were introduced in T170890 and I'm going to remove them.

Jdlrobson reopened this task as Open.Nov 16 2018, 12:43 AM
gerritbot added a comment.Nov 16 2018, 8:36 PM

Change 474327 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] QA: Drop more notification tests from Firefox

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

gerritbot added a comment.Nov 16 2018, 9:05 PM

Change 474327 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] QA: Drop more toast notification tests from Firefox (logged in search)

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

phuedx added a subscriber: phuedx.Nov 22 2018, 2:52 PM

@Jdlrobson: I've been bold and have tried to reflect what I think the state of this task is. It's hard for me to tell if this is indeed resolved because the browser tests are all failing with the Beta Cluster in read-only mode.

gerritbot added a comment.Nov 23 2018, 7:08 PM

Change 473939 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] QA: Remove hacks introduced for T170890

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

Jdrewniak added a subscriber: Jdrewniak.EditedNov 29 2018, 2:27 PM

Looking through the previous tasks, as well as the test itself, it seems like lots of stuff has been tried to fix this before. Not sure what would help, but one thing that I've identified is that the notification_area_element is the element with the selector .mw-notification-area. On the page, that element has a fixed position and a height of zero, so I'm not sure if selenium counts that as "visible".

Also, the toast itself is inside that element, so that leads me to question whether Selenium sees that child element as "visible" as well.

Jdrewniak added a comment.Nov 29 2018, 2:57 PM

Judging from this stackOverflow issue, this JS selenium implementation, and the GitHub issue below, it does seem that Selenium takes into account the height of an element to determine whether it is "shown".

This issue is there only in internet explorer. When one of the parent has a style property with height 0px and width 0px, selenium is telling the element is not visible.Hence it is not allowing actions on that element also. - https://github.com/SeleniumHQ/selenium/issues/5804

gerritbot added a comment.Nov 30 2018, 12:41 AM

Change 476790 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Drop toast tests from beta cluster

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

gerritbot added a comment.Nov 30 2018, 12:50 AM

Change 476790 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Drop toast tests from beta cluster

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

gerritbot added a comment.Nov 30 2018, 12:51 AM

Change 476791 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] QA: Drop protected page test

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

gerritbot added a comment.Nov 30 2018, 1:04 AM

Change 476791 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] QA: Drop protected page test

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

Jdlrobson moved this task from To Do to Ready for Signoff on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Nov 30 2018, 1:27 AM

We've tried a few things now. I'm open to changes to the selector, but I'm not convinced it will make much difference. Every time I see this, the toast does show, so I suspect Selenium is too slow to catch it, given its slow timeout.

I've re-tagged the tests to disable them. Feel free to revert this if you feel like that they are important, but right now I would much rather lose these tests on the beta cluster if it gets us more reliable browser tests.

pmiazga claimed this task.Dec 3 2018, 6:04 PM
pmiazga added a comment.Dec 3 2018, 10:38 PM

I'm still thinking about it. Honestly, I'm not a fan of this solution (the switch that disables failing test). I'll give it another shot, if I cannot get it working - then I'll resolve this task.

Jdlrobson added a comment.Dec 11 2018, 6:40 PM

Honestly, I'm not a fan of this solution (the switch that disables failing test).

I've only disabled those tests on the beta cluster. They will continue to run against Chrome for every single commit we submit to MobileFrontend.

Jdlrobson moved this task from Needs More Work to Ready for Signoff on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 12 2018, 7:59 PM

Talked with @pmiazga and I think we're both fine with resolving this now.

pmiazga added a comment.Dec 13 2018, 2:20 AM

yes, the problem happens only on the beta cluster and it's not easy to reproduce that locally.

pmiazga closed this task as Resolved.Dec 13 2018, 2:20 AM
pmiazga removed pmiazga as the assignee of this task.
pmiazga added a subscriber: pmiazga.
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL