Discarding an update doesn't clear the hamburger menu badge
Categories
(Toolkit :: Application Update, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox128 | --- | fixed |
People
(Reporter: bytesized, Assigned: ericchen647)
References
Details
(Whiteboard: [fidedi-ope])
Attachments
(2 files, 1 obsolete file)
STR:
- Install Nightly
- Navigate to about:preferences.
- Wait for "Update Now" badge to appear in the hamburger menu
- Select the Update option "Check for updates but let you choose to install them"
- Click "Discard" on the popup
Expected Results:
Hamburger menu badge is cleared or is replaced with "Update Available" badge
Actual Results:
Hamburger menu badge is unchanged.
|
Reporter | |
Updated•4 years ago
|
|
||
Updated•4 years ago
|
|
|
Reporter | |
Comment 1•3 years ago
|
||
I believe that Max is going to be taking this bug soon, so I'm going to go ahead and assign it now. And I'll leave a few pointers, mostly regarding testing.
I'm not sure if you've already been pointed towards UpdateListener.jsm, but that is where the update notification logic lives. I mentioned in the bug description that it would be reasonable to clear the badge or replace it with the "Update Available" badge. I'm still happy to do either, but I think that the former option is probably going to be easier. And the "Update Available" badge should still get shown the next time that we automatically check for updates anyways.
Unfortunately, I find it to be pretty common that adding testing is more of a pain than the original functionality change was.
I think that you haven't seen much of Firefox testing, so I'm going to give a quick primer on both Firefox testing in general and update testing in particular.
In general, tests need to be listed in .ini files in order to be run. I'll show browser_aboutPrefs_fc_downloadAuto_staging.js as an example, even though it is a bit of an unusual case. It is listed in two .ini files, browser.ini and browser.bits.ini. The reason for this duplication is that we run that test twice: once with BITS downloading enabled, once with it disabled. Most .ini files specify a head.js file, which runs before each test runs. Usually this defines useful testing functions and possibly does some test setup and/or teardown.
There are a whole bunch of different kinds of tests in the Firefox codebase. Update mostly just uses two of them: xpcshell tests and mochitests. xpcshell tests don't actually run the whole browser. Nothing graphical, for example, gets loaded. We use these types of tests to test the internals of the update system. Mochitests do actually load the whole browser. We can then run browser code directly, or we can use some special testing tools to navigate the browser and/or inject code into the content process.
Update testing mostly lives in toolkit/mozapps/update/tests/.
A lot of the common code for the update tests lives in the data subdirectory. Most of the files in there are support files like example update files. But some of them are a bit more noteworthy. app_update.sjs is the code for a little mock update server to test against. shared.js has a lot of functions that are shared amongst all the different tests. xpcshellUtilsAUS.js has update testing functions that are specific to xpcshell tests. These two shared code files are loaded by the head.js files for the relevant tests.
The update tests themselves live in a couple of different subdirectories:
browser- These are the update mochitests. All of the other directories contain xpcshell tests. These test the 2 update interfaces as well as update notifications.unit_aus_update- These are tests of the Application Update Service code.unit_background_update- These are tests that run the Background Update Agent code. They are currently pretty limited. We hope to expand on this at some point.unit_base_updater- These are tests of the updater binary that actually does the update installation.unit_service_updater- I believe that these tests are meant to be basically identical to the tests inunit_base_updater, but they have drifted a bit. These run the updater using the Mozilla Maintenance Service.
Since this test ought to test that a UI interaction causes a particular UI response, it definitely ought to be a mochitest in the browser directory. I think that this existing test would be good to model it after. In fact, probably the beginning of the new test will look nearly identical to the entirety of this test. Once it has got an update ready, you probably want to wait for the update badge to be shown, maybe like this. Then, you'll want to use SpecialPowers.spawn (or one of the similar functions) to run some code in the about:preferences page (runAboutPrefsUpdateTest should have left it open) to change the auto update setting. This test has some weirdness going on but it does change that setting, if you want an example to look at. Then you should be able to use BrowserTestUtils.promiseAlertDialog to wait for and interact with the "update in progress" dialog (which gets shown, here, btw). Then you should be able to check that the badge disappears similarly to how you check that it appeared.
Sorry that it's all a bit complicated. Let me know if you have any questions!
|
||
Comment 2•2 years ago
•
|
||
I've made some progress on the testing implementation for the hamburger menu update discard behavior and would appreciate your feedback!
My current approach is based on my understanding of Firefox's testing framework, which I've gained from reviewing numerous tests in the toolkits/mozapps/update/tests/browser directory. I've also specifically referenced browser_aboutPrefs_fc_downloadAuto_staging.js as per Robin's suggestion.
I'm not sure if this approach is redundant in certain respects, so I'll be glad to get some feedback!
I've named the new test file browser_aboutPrefs_fc_updateDiscard.js (Is this naming convention appropriate?)
Currently, I have three main test functions:
-
hamburger_menu_badge_update_available
-> Checks behaviour when an update is available.
-> Sets preference for the badge wait time to 0 (so that badge appears immediately when an update is available).
-> Opens a new tab to the about:preferences page, and checks for updates.
Waits for an update to be found, and then downloaded.
-> Uses a helper function to check the state of the badge on hamburger menu.
-> Expects badge to indicate that an update is available.
-> After test, closes the tab. -
hamburger_menu_badge_discard_update:
-> Checks behaviour when update is discarded.
-> Continues from state at end of previous test.
-> Simulates clicking Discard button.
-> Uses helper function to check badge state on hamburger menu.
-> Expects badge to be cleared. -
hamburger_menu_badge_update_reappear:
-> Checks behaviour when new update is available after prev update was discarded.
-> As before, continues from state at end of previous test.
-> Waits for new update to be found, and downloaded.
-> After update downloaded, uses helper function to check badge state on hamburger menu.
-> Expects the badge to reappear showing Update Available, even though previous update was discarded.
My helper function getHamburgerMenuBadgeState checks the badge state on the hamburger menu.
I'd appreciate your feedback here:
Are there alternative ways I could approach these tests?
Is the badge behavior as I've implemented it (clearing on discard, then refreshing when a new update is available) the desired functionality? (I've taken some liberty here)
I'm also a bit concerned about possibly being redundant by checking things in too much detail. Do you feel this is the case, or is the level of detail appropriate for these tests?
Open to exploring other approaches and would welcome any recommendations. Thank you!
|
|
||
Comment 3•2 years ago
|
||
|
|
||
Comment 4•2 years ago
|
||
|
||
Updated•2 years ago
|
I'm trying to follow the STR to replicate the bug - do I have to install an earlier version of Nightly (is that possible?), because otherwise Nightly will already be up to date so the "Update Now" badge is not displayed.
|
|
Reporter | |
Comment 6•1 year ago
|
||
There are probably two reasonable ways to reproduce this.
You can browse through archive.mozilla.org to find an old version of Nightly. I would suggest something recent, without getting the very newest version.
You can also set up a local update server and force a locally built copy of Firefox to install an update. If you build an update MAR from the locally built copy, you can even continuously update from the locally built version to the locally built version. This method is more complicated than using Nightly, but I would recommend it because it allows you to run against your local copy of the code, allowing you to test your fix before landing it.
|
|
||
Updated•1 year ago
|
|
||
Comment 9•1 year ago
|
||
| bugherder | ||
Description
•