1304656 - Change update tests to use the restart button in the about and software update window to restart Firefox

Status: RESOLVED FIXED

(Testing :: Firefox UI Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With bug 1298800 being fixed soon, we can finally use a callback to restart Firefox. With that feature we can simulate a click on the restart button inside the about and software update window.

Therefore we would need the following things:

1. Add element references to the restart button for the appropriate deck in the about and software update deck classes (use the DOM inspector to retrieve those):

https://dxr.mozilla.org/mozilla-central/source/testing/puppeteer/firefox/firefox_puppeteer/ui/about_window/deck.py

https://dxr.mozilla.org/mozilla-central/source/testing/puppeteer/firefox/firefox_puppeteer/ui/update_wizard/dialog.py

2. Pass in a click on the appropriate restart button via a parameter to `self.restart()`. This applies to the following calls:

https://dxr.mozilla.org/mozilla-central/source/testing/firefox-ui/harness/firefox_ui_harness/testcases.py#319
https://dxr.mozilla.org/mozilla-central/source/testing/firefox-ui/harness/firefox_ui_harness/testcases.py#362

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I would like to have this as part of the next puppeteer release. So marking as blocking bug 1313312, which will get a new one out.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1304656 - Firefox UI update tests have to use the restart buttons.

Review commit: https://reviewboard.mozilla.org/r/89252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89252/

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8805507 [details]
Bug 1304656 - Firefox UI update tests have to use the restart buttons.

https://reviewboard.mozilla.org/r/89250/#review88438

Rail, going forward with update tests I think it would be good if you understand more of the tests. So I hope it is ok for you to pass you along some easy to review changes. That way you can build up your knowledge.

Comment 4

[Rail Aliiev [:rail]]

Comment on attachment 8805507 [details]
Bug 1304656 - Firefox UI update tests have to use the restart buttons.

https://reviewboard.mozilla.org/r/89252/#review89644

LGTM

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/775eb155363c
Firefox UI update tests have to use the restart buttons. r=rail

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/775eb155363c

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can we please get this patch backed out from mozilla-central and mozilla-aurora? It causes a regression for our update tests when the source build's version is less than 52. Given that we do not gain any benefits from this patch and the updater UI might change soon, it doesn't outweigh carrying workaround with us the next releases. Thanks.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Hitting conflicts because of https://hg.mozilla.org/mozilla-central/rev/c78bef5d7961a10ed0333aaf0b045a5bcd0697b9

Could we get either a rebased backout patch or should I just back that other patch out too?

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I will provide a backout patch. We should not backout the other referenced patch for it.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: backout patch (central, aurora)

This patch reverses the changes and takes care of the filename and code changes. It has to work for central and aurora.

Thanks Wes for backing it out for us.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backouts:
https://hg.mozilla.org/mozilla-central/rev/79feeed4293336089590320a9f30a813fade8e3c
https://hg.mozilla.org/releases/mozilla-aurora/rev/d986af3df3388f93fda33ab992e1db8b69e167e8

Comment 12

[Sankha Narayan Guria [:sankha]]

Attachment: Back out revision 775eb155363c () for causing regressions in update tests

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can anyone please tell me why this patch has been backed out? I do not see anything related to failures for update tests. Based on which decision did the backout happen?

Also please do not change the assignee of the bug where people are actively working on.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Sankha Narayan Guria [:sankha] from comment #12)
> Created attachment 8811142 [details] [diff] [review]
> Back out revision 775eb155363c () for causing regressions in update tests

Hm, I think that I was confused by the double mentioning of backouts. So for me it gave the assumption that the backout got backed out. Maybe we can make that a bit clearer in the future. What I still do not understand is the additionally added attachment to this bug. What is it for?

Comment 15

[Sankha Narayan Guria [:sankha]]

(In reply to Henrik Skupin (:whimboo) from comment #13)
> Can anyone please tell me why this patch has been backed out? I do not see
> anything related to failures for update tests. Based on which decision did
> the backout happen?
> 
> Also please do not change the assignee of the bug where people are actively
> working on.

Hey, I am sorry. This looks like another instance of comments that got sent out because of my misbehaving bzexport mercurial extension just like bug 1313595. I haven't backed out anything, just that changeset was attached here as an attachment.

Sorry for the noise!

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks!

Comment 17

[Julien Cristau [:jcristau] (back April 22)]

Too late for firefox 52, mass-wontfix.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Sadly we missed to re-land this patch for the 53.0 time frame. So lets do it now for 55 and 54. I will update the patch and get it pushed.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8805507 [details]
Bug 1304656 - Firefox UI update tests have to use the restart buttons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89252/diff/1-2/

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Last mozreview update just fixes the merge conflicts for current mozilla-central. No further changes have been made.

Sadly I cannot test this patch because it's blocked by a regression from bug 1356154, which needs to be fixed first.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1304656 - Puppeteers restart method has to pass-through arguments.

Puppeteer enforces to use an in_app restart, unless the clean argument is
set to True. But whatever case is present, all passed in arguments have
to be forwarded to Marionette.

Review commit: https://reviewboard.mozilla.org/r/131396/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131396/

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8805507 [details]
Bug 1304656 - Firefox UI update tests have to use the restart buttons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89252/diff/2-3/

Comment 23

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8859372 [details]
Bug 1304656 - Puppeteers restart method has to pass-through arguments.

https://reviewboard.mozilla.org/r/131396/#review133980

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a73bf50396ab
Puppeteers restart method has to pass-through arguments. r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/654319f02572
Firefox UI update tests have to use the restart buttons. r=rail

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a73bf50396ab
https://hg.mozilla.org/mozilla-central/rev/654319f02572

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Test-only change which gets our tests closer to what the user is doing. Please uplift to beta. Thanks.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b992c7c7fa04
https://hg.mozilla.org/releases/mozilla-beta/rev/ee39b53a33c0

Comment 28

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

So it seems this fix is causing bug 1317707 to show again - but only when updating from Firefox 51 - see the ondemand update test results for 54b2 [1] (all updates from Fx 51 fail with bug 1317707).

Searching for a regression window I saw that the issue appeared between:
- Last good - https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-macosx64/1493039179/firefox-54.0.en-US.mac.test_packages.json
- First bad - https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-macosx64/1493062585/firefox-54.0.en-US.mac.test_packages.json

This points to the following pushlog (which contains the fix for this bug): https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=d5fe7a284cf1&tochange=a236801aa022

Since this affects only update tests from Firefox 51 (which pass with an older test package), I'm tempted to say that we could in fact ignore it... just thought I should mention it in case it matters - @whimboo; maybe it will affect ESR (updates from 45 to 52)?

[1] - https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=ca3c6131b6ea&group_state=expanded&filter-tier=3&filter-searchStr=Fxup-beta-cdntest(

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh, thanks Florin for bringing this up! I totally missed that the 51 release is still a truly supported release for our update tests to version 54. So the uplift was indeed inappropriate, and should be reverted.

Sheriffs, can one of you please backout both commits from the mozilla-beta branch? Thanks.

Comment 30

[Carsten Book [:Tomcat]]

backout done in 

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/c71323755db2d14dacee7c274ef0160aaf5124da
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/a1ff0d20ca0b924f2cb98f9b99dbb884114d7bd7

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thank you Carsten! So this is clearly a wontfix for 54.0.