1907126 - Create tests for delay updates with multiple instances and multiple profiles

Status: RESOLVED FIXED

(Toolkit :: Application Update, task, P3)

Description

[Max Christian Pohle [:/dev/max]]

To ensure the safety of:

• timestamp file
• entry points
• launch test with various configurations

Comment 1

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1907126 - Refactor out repeated update testing code r=nrishel!,nalexander!

The code to start an update download seems to repeated multiple times in a bunch of different places. This patch is going to consolidate it so that the next patch in the stack can just tweak that function and call into it rather than writing yet another implementation of almost the same thing.

Additionally, the code to start an app_update.sjs update server now exists in at least two xpcshell tests. This patch consolidates them into a single implementation: startSjsServer. Hopefully this is sufficiently distinct from start_httpserver, which starts a different type of server. In addition to moving the implementation, it has also been changed to use random ports rather than using the same hardcoded port number every time. This is important since xpcshell tests can run in parallel.

Comment 2

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1907126 - Split `--test-process-updates` functionality r=nrishel!,nalexander!

--test-process-updates currently basically does two totally different things: 1) It facilitates testing applying updates via the application startup process by preventing the application from actually starting if the update isn't applied or when it is re-launched as the callback application, and 2) If ShouldNotProcessUpdates finds a reason not to process updates, it does nearly the opposite: it sets the env variable to represent the reason we didn't process updates and lets the application continue to launch in order for a background task to consume that value.

This did not previously cause problems because we never did any testing that resulted in finding a reason not to process updates at startup that didn't involve also involve starting a background task. But the test later in this stack needs to do exactly that, motivating this change.

So this patch will split out functionality (2) into --test-should-not-process-updates so that the --test-process-updates behavior can consistently be: only process updates, do not launch the browser.

Comment 3

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1907126 - Add testing for the multi session install lock feature r=nrishel!,nalexander!

Additionally disables the multi session install lock feature by default in update tests so that it doesn't interfere with tests not meant to test that behavior.

This does add telemetry testing, but does not do it as part of the main feature test. The problem with testing the telemetry in that test is that something has to stick around after startup to observe the telemetry. It is possible to have the update applied by a background task rather than a regular instance of the application and then to have that background task observe the telemetry. The problem is that applying the updates with a background task screws up the test because we are making sure that, in some cases, updates are applied at startup even if another instance is running. But background tasks are forbidden from doing exactly that. So the telemetry testing for this feature instead lives in the shouldprocessupdates background task used by toolkit/components/backgroundtasks/tests/xpcshell/test_backgroundtask_shouldprocessupdates.js.

Comment 4

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1907126 - Make common test functionality more self contained r=nrishel!,nalexander!

The test added in this stack doesn't really do anything new, but it does do some things repeatedly that had previously only been done once per test. This patch is largely aimed at making that functionality more self contained so that it doesn't break or work inconsistently when repeated.

The biggest of these changes is to the --test-process-updates behavior. Previously, it would exit the browser after all update processing is complete. Now it additionally writes its PID into the update directory upon completion. This makes it more reasonable to make runUpdateUsingApp self contained, as there were problems where parts of the app were still running after this function exited, causing problems.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/636054838661
Refactor out repeated update testing code r=nrishel,nalexander,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/6cec10a15827
Split `--test-process-updates` functionality r=nrishel,nalexander
https://hg.mozilla.org/integration/autoland/rev/c500e441e887
Add testing for the multi session install lock feature r=nrishel,nalexander,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/7a9ba6ef7133
Make common test functionality more self contained r=nrishel,nalexander,application-update-reviewers

Comment 6

[Cristina Horotan [:chorotan]]

Backed out for causing build bustages

Backout link

Push with failures

Failure log

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5f433a476de
Refactor out repeated update testing code r=nrishel,nalexander,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/2c26ce3f8dd3
Split `--test-process-updates` functionality r=nrishel,nalexander
https://hg.mozilla.org/integration/autoland/rev/0743d5998d6a
Add testing for the multi session install lock feature r=nrishel,nalexander,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/a4acaa6581bb
Make common test functionality more self contained r=nrishel,nalexander,application-update-reviewers

Comment 8

[amarc]

https://hg.mozilla.org/mozilla-central/rev/b5f433a476de
https://hg.mozilla.org/mozilla-central/rev/2c26ce3f8dd3
https://hg.mozilla.org/mozilla-central/rev/0743d5998d6a
https://hg.mozilla.org/mozilla-central/rev/a4acaa6581bb