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

RESOLVED FIXED in Firefox 55

Status

Testing
Firefox UI Tests
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

(Blocks: 1 bug)

Version 3
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 wontfix, firefox-esr52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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
(Assignee)

Updated

2 years ago
Blocks: 1303834
(Assignee)

Comment 1

2 years ago
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.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Comment 3

2 years ago
mozreview-review
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.
(Assignee)

Updated

2 years ago
Attachment #8805507 - Flags: review?(rail)

Comment 4

2 years ago
mozreview-review
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
Attachment #8805507 - Flags: review?(rail) → review+

Comment 5

2 years ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/775eb155363c
Firefox UI update tests have to use the restart buttons. r=rail

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/775eb155363c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
(Assignee)

Updated

2 years ago
Depends on: 1317707
(Assignee)

Comment 7

2 years ago
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.
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?
Flags: needinfo?(hskupin)
(Assignee)

Comment 9

2 years ago
I will provide a backout patch. We should not backout the other referenced patch for it.
Flags: needinfo?(hskupin)
(Assignee)

Comment 10

2 years ago
Created attachment 8811047 [details] [diff] [review]
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.
Flags: needinfo?(wkocher)
Backouts:
https://hg.mozilla.org/mozilla-central/rev/79feeed4293336089590320a9f30a813fade8e3c
https://hg.mozilla.org/releases/mozilla-aurora/rev/d986af3df3388f93fda33ab992e1db8b69e167e8
Status: RESOLVED → REOPENED
status-firefox52: fixed → affected
status-firefox53: --- → affected
Flags: needinfo?(wkocher)
Resolution: FIXED → ---
Created attachment 8811142 [details] [diff] [review]
Back out revision 775eb155363c () for causing regressions in update tests
Assignee: hskupin → sankha93
Status: REOPENED → ASSIGNED
(Assignee)

Comment 13

2 years ago
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.
Assignee: sankha93 → hskupin
Flags: needinfo?(wkocher)
Flags: needinfo?(sankha93)
(Assignee)

Comment 14

2 years ago
(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?
Whiteboard: [lang=py] → [lang=py][needs minimum supported version of 52 for update tests]
(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!
Flags: needinfo?(sankha93)
Attachment #8811142 - Attachment is obsolete: true
(Assignee)

Comment 16

2 years ago
Thanks!
Flags: needinfo?(wkocher)
(Assignee)

Updated

2 years ago
Mentor: hskupin
Whiteboard: [lang=py][needs minimum supported version of 52 for update tests] → [needs minimum supported version of 52 for update tests]
Too late for firefox 52, mass-wontfix.
status-firefox52: affected → wontfix
(Assignee)

Updated

a year ago
Blocks: 1355818
(Assignee)

Comment 18

a year ago
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.
status-firefox53: affected → wontfix
status-firefox54: --- → affected
status-firefox55: --- → affected
status-firefox-esr52: --- → wontfix
Whiteboard: [needs minimum supported version of 52 for update tests]
(Assignee)

Updated

a year ago
Depends on: 1356154
Comment hidden (mozreview-request)
(Assignee)

Comment 20

a year ago
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 hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 23

a year ago
mozreview-review
Comment on attachment 8859372 [details]
Bug 1304656 - Puppeteers restart method has to pass-through arguments.

https://reviewboard.mozilla.org/r/131396/#review133980
Attachment #8859372 - Flags: review?(mjzffr) → review+

Comment 24

a year ago
Pushed by hskupin@mozilla.com:
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
(Assignee)

Updated

a year ago
Blocks: 1298803

Comment 25

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a73bf50396ab
https://hg.mozilla.org/mozilla-central/rev/654319f02572
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years agoa year ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
(Assignee)

Comment 26

a year ago
Test-only change which gets our tests closer to what the user is doing. Please uplift to beta. Thanks.
Whiteboard: [checkin-needed-beta]
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(
Flags: needinfo?(hskupin)
(Assignee)

Comment 29

a year ago
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.
Flags: needinfo?(hskupin)
Target Milestone: mozilla52 → mozilla55
(Assignee)

Updated

a year ago
Flags: needinfo?(sheriffs)
Flags: needinfo?(sheriffs)
(Assignee)

Comment 31

a year ago
Thank you Carsten! So this is clearly a wontfix for 54.0.
status-firefox54: affected → wontfix
You need to log in before you can comment on or make changes to this bug.