1707753 - 6 tests skipped on apple_silicon

Status: REOPENED

(Toolkit :: Application Update, task, P3)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

Last week we turned on tests for Apple Silicon (OSX 11.2.3 on new Apple based hardware). We are using the simplified new test config process:
https://firefox-source-docs.mozilla.org/testing/ci-configs/index.html

As the tests are live, we are now filing bugs to help close the loop and hope to fix any issues over the next 7 weeks. As the process outlines, there are tier-3 jobs running on m-c which run these skipped tests and will expect a failure/timeout/crash- if it doesn't fail, then the job will turn orange.

Here are some failing tests:
toolkit/components/taskscheduler/tests/xpcshell/test_TaskSchedulerMacOSImpl.js
toolkit/components/taskscheduler/tests/xpcshell/test_TaskScheduler.js

and some more failing tests:
toolkit/mozapps/update/tests/unit_base_updater/marStageSuccessComplete.js
toolkit/mozapps/update/tests/unit_base_updater/marStageSuccessPartial.js
toolkit/mozapps/update/tests/unit_base_updater/marAppInUseStageSuccessComplete_unix.js
toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateStageSuccess.js

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:nalexander - I am giving you a heads up as triage owner that these tests are skipped on the new apple silicon platform.

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Joel Maher ( :jmaher ) (UTC -0800) from comment #1)

:nalexander - I am giving you a heads up as triage owner that these tests are skipped on the new apple silicon platform.

Thanks jmaher. The task scheduler ones look like some differences in configuration.

I can't explain the unit_base_updater failures -- Kirk, any ideas?

Comment 3

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

It looks to me like the unit_base_updater tests are all failing the same way. They effectively time out after hitting this error:

JavaScript error: /opt/worker/tasks/task_161911974565346/build/tests/xpcshell/head.js, line 241: uncaught exception:
Waiting for file to exist, path: /opt/worker/tasks/task_161911974565346/build/tests/xpcshell/tests/toolkit/mozapps/update/tests/unit_base_updater/marStageSuccessComplete/dir.app/Contents/Resources/postup_app.log - timed out after 50 tries.

I had to do some digging to figure out what the story with that is. All three of these tests call checkPostUpdateAppLog(), which checks the contents of postup_app.log. In testing, we override PostUpdate by changing updater.ini, which specifies what ought to be called for PostUpdate. I believe that the binary specified is TestAUSHelper, which I think is copied to that location here. That binary ought to write to postup_app.log here. But the error message suggests that either it isn't, or there is a long enough delay that we time out waiting for it.

It's pretty hard to pinpoint where the failure is happening here. The log shows that we are reading a success value from update.status, but I believe that that still gives us anywhere between the invocation of LaunchCallbackAndPostProcessApps and the writing of the log file for something to go wrong that would result in the file not being written. This leaves us with a pretty large number of things that could be going wrong to cause this.

I'm not entirely sure how to proceed here. I don't have access to any relevant hardware, and I'm not sure whether it would be worth it for me to get hardware for this and set it all up just to figure this out. Adding a lot more logging and re-running on try is the best option I can think of, but it would still be pretty tricky. The logging that I would need would be in the updater binary, and AFAIK I can't easily get at the updater logs generated by a test. Maybe I could add some code to the test to read the logs to include them as part of the test log? This sounds like something I would likely need to dedicate some time to.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for the digging in :bytesized. As a note, we have turned off these tests temporarily while we work to get things running in arm64 mode instead of emulated in x86_64- also we need to upgrade the machines for security issues. Having this data when we turn the tests back on will help validate things, test things differently, or confirm we are still in the same boat.

Comment 5

[[e-mail] (:bhearsum)]

Newer logs for this are available on https://treeherder.mozilla.org/jobs?repo=mozilla-central&tier=1%2C2%2C3&searchStr=macosx1100&test_paths=toolkit%2Fmozapps%2Fupdate%2Ftests%2Funit_base_updater&selectedTaskRun=bN5NfSCESuubbzrtos0O5Q.0

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1707753 - Mark copied app files as unquarantined in macOS updater unit tests. r?bytesized

Newer versions of macOS mark copied applications as quarantined and
pop a UI dialog when they are run. It's possible to manually
unquarantine them by removing specific extended attributes. It
appears the attributes must be removed from directories and files.

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1707753 - Use `Subprocess.jsm` to launch application, rather than `nsIProcess` + `/bin/sh`. r?bytesized

The existing code works around two limitations of nsIProcess:
there's no support for output redirection and the environment is
inherited from the parent directly. Subprocess.jsm is not limited
in these ways.

I hypothesize that the existing /bin/sh launch mechanism fails due
to changes to the shell serving /bin/sh shebangs that rolled out
with macOS 10.15 (Catalina). These failures do not reproduce locally
for me (with an M1 with macOS 11.0). The invocations are silent and
this should at least provide logging when they fail.

Depends on D148559

Comment 8

[Nick Alexander :nalexander [he/him]]

Kirk: throwing out these two patches that I worked up during a deep dive into this ticket moons ago. I'm quite sure they didn't actually address the issue, and I never could repro on an actual M1 device.

Try build is percolating at https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f1a3bce4216f591ea663fa7092fff8d7c15a5fc.

We might want to land these if they're neutral, even if they don't address the issue, since they set the stage. Or we might wait until we get a loaner M1 to repro in automation.

Comment 9

[Nick Alexander :nalexander [he/him]]

I rebased and pushed a new try build at https://treeherder.mozilla.org/jobs?repo=try&revision=11e5fa13b9cc87535b541d6d8588ab9040258f07. We'll see what happens.

Comment 10

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander [he/him] from comment #9)

I rebased and pushed a new try build at https://treeherder.mozilla.org/jobs?repo=try&revision=11e5fa13b9cc87535b541d6d8588ab9040258f07. We'll see what happens.

It looks like this is enough: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=TSWgobErRV6NY6qN2CofeQ.0&revision=11e5fa13b9cc87535b541d6d8588ab9040258f07. I'll work with :bytesized to get this landed.

Comment 11

[Nick Alexander :nalexander [he/him]]

OK, this has been an odyssey, but I'm getting closer. First, the earlier try build didn't actually run any tests due to a weirdness in test-verify. But https://treeherder.mozilla.org/jobs?repo=try&revision=5948f297f4294f7df361df6c7174811cebf0bea5 actually does run the macOS tests, and they're green. However, there's a different unexpected interaction with the Subprocess.sys.mjs changes: the tests succeed locally with XRE_PROFILE_PATH set, but fail with -profile only. That's counter to my expectations, so I'm going to try to run it down.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4750f3371d2d
Mark copied app files as unquarantined in macOS updater unit tests. r=bytesized,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/f2cbd63247d4
Use `Subprocess.jsm` to launch application, rather than `nsIProcess` + `/bin/sh`. r=bytesized,application-update-reviewers

Comment 13

[Cristian Tuns]

Backed out for causing xpcshell failures in marAppApplyUpdateAppBinInUseStageSuccess_win.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateAppBinInUseStageSuccess_win.js | runUpdateUsingApp - [runUpdateUsingApp : 4579] the status file state should equal the expected value - "applied" == "succeeded"

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c225644025f
Mark copied app files as unquarantined in macOS updater unit tests. r=bytesized,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/85a01700b713
Use `Subprocess.jsm` to launch application, rather than `nsIProcess` + `/bin/sh`. r=bytesized,application-update-reviewers

Comment 15

[Cristian Tuns]

Backed out for causing xpcshell failures in marAppApplyUpdateAppBinInUseStageSuccess_win.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateAppBinInUseStageSuccess_win.js | runUpdateUsingApp - [runUpdateUsingApp : 4613] the status file state should equal the expected value - "applied" == "succeeded"

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/624787a1c115
Mark copied app files as unquarantined in macOS updater unit tests. r=bytesized,application-update-reviewers

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/624787a1c115

Comment 18

[Sandor Molnar[:smolnar]]

Backed out for causing bug 1877654
Backout link: https://hg.mozilla.org/mozilla-central/rev/ab51486dc9d2481ae051756701737c342b624b0d

Comment 19

[Stephen A Pohl [:spohl]]

After bug 1827651 lands, this will get enabled in bug 1910401 (pending reviews and feedback).