599233 - Create tests for update checking in the Firefox about window

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robert Strong (they/them - no direct email)]

Spinoff of bug 596813 for tests so bug 596813 can be closed

Comment 1

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

Rob, do you have an ETA for this bug? If it will take a while I would jump in and update our Mozmill tests as soon as possible to support the new ui.

Comment 2

[Robert Strong (they/them - no direct email)]

Not at the moment

Comment 3

[Robert Strong (they/them - no direct email)]

It's unlikely that I will get to work on this in the next two weeks, so unassigning myself for now.

Comment 4

[Robert Strong (they/them - no direct email)]

Attachment: patch - minor client changes

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch - tests

Comment 6

[Robert Strong (they/them - no direct email)]

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1c6e21b65899e851d7118f3bc892abc12b04aa6

I am trying to come up with a naming scheme for the tests so it is easier to know what they do and to make it easier to associate them with tests that are the same except for a few details. So far I have

fc = foreground check
bc = background check
sp = startup processing
check = only the update check was performed (also keeps these tests sorted together)
downloadAuto = an update download was performed automatically (app.update.auto = true)
downloadOptIn = the client chose to download an update (app.update.auto = false)
downloaded = an update was already downloaded automatically (app.update.auto = true)
patch = patch condition tests (also keeps these tests sorted together)

Comment 7

[Robert Strong (they/them - no direct email)]

Attachment: patch - make a copy of the update.sjs file for chrome tests

Since the chrome tests will be going away soon I just made a copy of the update.sjs file instead of trying to fix the chrome tests to work with the changes needed for the browser-chrome tests.

Comment 8

[Robert Strong (they/them - no direct email)]

Attachment: patch - tests

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=affaa4fe7ffa9d5526d245f19ae696a2b08238d8

Based on the last push I suspect that a couple of tests fail on debug builds.

Comment 9

[Robert Strong (they/them - no direct email)]

Attachment: patch - copy and rename of the update.sjs file

I put the file copy and rename in this patch so it is easier to read this change as well as the changes in the next patch.
I copied the test http server handler (update.sjs) to chrome so these changes wouldn't affect the chrome tests since these chrome tests will (hopefully) be going away in the near future.
I moved update.sjs to the browser directory since it will be the only consumer and I renamed it because searching for update.sjs returns unrelated results.

Comment 10

[Robert Strong (they/them - no direct email)]

Attachment: patch - tests

The test naming scheme I am using is in comment #6. I plan on changing the existing browser tests to this naming scheme in another bug that I haven't filed yet.

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5aa18db4e5a8dc05ec70e1ab1e660c80dd43fe08

Comment 11

[Robert Strong (they/them - no direct email)]

Attachment: patch -tests

Missed an eslint error due to a last minute change

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f9fd5de113d9dd00023a87bf555cbb86804217c

Comment 12

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9036828 [details] [diff] [review]
patch -tests

Review of attachment 9036828 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/update/tests/browser/app_update.sjs
@@ +70,3 @@
>        if (continueFile.exists()) {
> +        try {
> +          // If the continue file is in use try again the time the timer fires.

typo: "the _next_ time the timer fires."

@@ +170,5 @@
> +    gSlowCheckTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
> +    gSlowCheckTimer.initWithCallback(function(aTimer) {
> +      if (continueFile.exists()) {
> +        try {
> +          // If the continue file is in use try again the time the timer fires.

Same typo.

Comment 13

[Robert Strong (they/them - no direct email)]

Attachment: patch - tests with comments addressed

Carrying forward r+

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1293016267a7
add about dialog app update tests. r=mhowell

Comment 15

[Robert Strong (they/them - no direct email)]

Attachment: combined patch as landed

Comment 16

[Noemi Erli[:noemi_erli]]

Backed out changeset 1293016267a7 (Bug 599233) for failing in browser_preferences.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=1293016267a73f6408b93c203e8b28242b2e2be6&selectedJob=222315811

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=222315811&repo=mozilla-inbound&lineNumber=4366

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/913880b6d5c60c9fc4c2484036ca37c633bcd2b9

Comment 17

[Robert Strong (they/them - no direct email)]

Comment on attachment 9036510 [details] [diff] [review]
patch - minor client changes

This is what caused the screenshots test failures so I am not going to land this at this time.

Comment 18

[Robert Strong (they/them - no direct email)]

Attachment: patch - tests

Changed the browser.ini as follows
[browser_about_bc_downloaded_staged.js]
skip-if = (os == "linux" && verify) || asan

Filed bug 1520672 for the Linux TV failures.

Comment 19

[Robert Strong (they/them - no direct email)]

I got an rs from mhowell for disabling the test for TV runs. It appears that during TV runs this test is triggering bug 1388764.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc1733154e3d
add about dialog app update tests. r=mhowell

Comment 21

[Robert Strong (they/them - no direct email)]

Attachment: combined patch as landed

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee96fa31af70
followup to disable staging tests when running --verify on linux due to this likely causing a crash in TV1 runs. r=me

Comment 23

[Robert Strong (they/them - no direct email)]

Attachment: followup patch - disable TV runs (e.g. --verify) for staging tests on Linux

Comment 24

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/cc1733154e3d
https://hg.mozilla.org/mozilla-central/rev/ee96fa31af70