Closed Bug 1506917 Opened 4 years ago Closed 4 years ago

Don't attempt to check for updates from Firefox's nsBrowserGlue.js when running tests

Categories

(Toolkit :: Application Update, enhancement)

59 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

Attachments

(1 file, 2 obsolete files)

I noticed that when running tests that nsBrowserGlue.js was attempting to check for an update. This can be disabled by setting the app.update.checkInstallTime bool pref to false.
Attached patch patch rev1 (obsolete) — Splinter Review
Comment on attachment 9024818 [details] [diff] [review]
patch rev1

Hi Ted, I noticed this when running tests locally. Basically, Firefox has an option to perform a background check in nsBrowserGlue.js that will run when the build ID is 2 days or greater old. Though the disabledForTesting pref prevents it from performing the update check it still hits the app update telemetry code so I'd like to disable this when running tests.
Attachment #9024818 - Flags: review?(ted)
Comment on attachment 9024818 [details] [diff] [review]
patch rev1

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

::: testing/geckodriver/src/prefs.rs
@@ +7,1 @@
>  // so please be careful and get review from a Testing :: geckodriver peer

I don't know that I qualify as a geckodriver peer for these purposes. :)
Attachment #9024818 - Flags: review?(ted) → review+
Holy smokes we still have a lot of places that define testing prefs!
Attached patch patch rev2 (obsolete) — Splinter Review
I removed the change to prefs.rs for geckodriver... I'll file a separate bug for that change.

Carrying forward r+
Attachment #9024818 - Attachment is obsolete: true
Attachment #9026097 - Flags: review+
Filed bug 1508283 for geckdriver
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1c306693151
Don't attempt to check for updates from Firefox's nsBrowserGlue.js when running tests. r=ted
Attached patch patch rev2Splinter Review
Patch as landed... had to unbitrot it
Attachment #9026097 - Attachment is obsolete: true
Attachment #9026104 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/b1c306693151
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Comment on attachment 9026104 [details] [diff] [review]
patch rev2

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

::: testing/marionette/components/marionette.js
@@ +59,5 @@
>    // Disable automatically upgrading Firefox.
>    //
>    // This should also be set in the profile prior to starting Firefox,
>    // as it is picked up at runtime.
> +  ["app.update.checkInstallTime", false],

This is a preference which needs to be set during start-up, right? So it should only have been set in geckoinstance.py, but not marionette.js. The latter is even only related to geckodriver and should not have been modified here but at least on bug 1508283. Sadly we are getting into an inconsistent state of prefs again because patches land without our review. :(
Flags: needinfo?(robert.strong.bugs)
It is needed during startup. I added it to testing/marionette/components/marionette.js because it had the app.update.disabledForTesting pref along with this comment

https://searchfox.org/mozilla-central/source/testing/marionette/components/marionette.js#61

  // This should also be set in the profile prior to starting Firefox,
  // as it is picked up at runtime.

It looks like you can also remove app.update.disabledForTesting since that should be set during startup.

In the case of geckodriver.py I got a geckodriver peer to review in bug 1508283 because of a comment in the code per comment #4. Perhaps you can do something similar and let the testing owner (ted) and other testing peers know. I've also had to do something similar in 
https://searchfox.org/mozilla-central/source/browser/installer/removed-files.in

and I periodically get pinged from people trying to add to it.
Flags: needinfo?(robert.strong.bugs)
Thanks Robert! I will take care of it on bug 1509256, which will improve the documentation, and removes both preferences from marionette.js.
Blocks: 1509256
You need to log in before you can comment on or make changes to this bug.