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

Project	Branch	Lines +/-	Subject
mediawiki/skins/MinervaNeue	master	+0 -10	QA: Drop protected page test
mediawiki/skins/MinervaNeue	master	+5 -5	Drop toast tests from beta cluster
mediawiki/skins/MinervaNeue	master	+3 -9	QA: Remove hacks introduced for T170890
mediawiki/skins/MinervaNeue	master	+1 -1	QA: Drop more toast notification tests from Firefox (logged in search)
mediawiki/skins/MinervaNeue	master	+2 -2	QA: Drop some tests in Firefox

Customize query in gerrit

Related Objects

Mentioned Here: rMW886c37e94f6c: mediawiki.notification: Use data-mw-foo instead of data-mw.foo
T170890: [4 hrs] MinervaNeue browser test are flaking (waiting for {:class=>"mw-notification", :tag_name=>"div"} to become present )

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)

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:

times out waiting for the toast to appear
Problem with css selector div(:toast, css: '.mw-notification')

Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.Nov 5 2018, 10:42 PM

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.

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

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 project: Patch-For-Review.Nov 15 2018, 8:53 PM

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

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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.6; 2018-11-27).Nov 15 2018, 10:00 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

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

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

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 edited projects, added Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2); removed Readers-Web-Backlog.Nov 22 2018, 2:49 PM

phuedx moved this task from To Do to Ready for Signoff on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.

@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.

Jdlrobson moved this task from Ready for Signoff to Blocked on Others on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Nov 22 2018, 3:14 PM

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

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

Sadly this is still not fixed.. https://integration.wikimedia.org/ci/view/Reading-Web/job/selenium-MinervaNeue/713/BROWSER=chrome,MEDIAWIKI_ENVIRONMENT=beta,PLATFORM=Linux,label=BrowserTests/

Jdlrobson moved this task from To Do to Blocked on Others on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Nov 28 2018, 6:22 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.

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

Still seeing this issue in build 728 https://integration.wikimedia.org/ci/view/Reading-Web/job/selenium-MinervaNeue/728/BROWSER=chrome,MEDIAWIKI_ENVIRONMENT=beta,PLATFORM=Linux,label=BrowserTests/testReport/junit/(root)/Manage%20Watchlist/Add_an_article_to_the_watchlist/

Jdlrobson moved this task from Blocked on Others to To Do on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Nov 29 2018, 11:41 PM

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

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

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

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

ReleaseTaggerBot edited projects, added MW-1.33-notes (1.33.0-wmf.8; 2018-12-11); removed MW-1.33-notes (1.33.0-wmf.6; 2018-11-27).Nov 30 2018, 1:00 AM

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

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

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

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.

pmiazga moved this task from Ready for Signoff to Needs More Work on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 3 2018, 10:38 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.

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

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.

[Spike, 2hrs] Identify the problem with the toast notification tests on the beta clusterClosed, ResolvedPublicActions