1614626 - Move xpcshell selftests out of builds

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, enhancement, P3)

Description

[Marco Castelluccio [:marco]]

It's currently painful to debug a crash in a xpcshell selftest, as the minidumps are not processed and they are not uploaded as artifacts either.

MINIDUMP_STACKWALK not set, can't process dump.

I imagine we need to set the MINIDUMP_STACKWALK environment variable (https://searchfox.org/mozilla-central/rev/a1592902acabf9bded973067133baaac1457f3d3/testing/mozbase/mozcrash/mozcrash/mozcrash.py#205) in order to process the minidumps.

And, we need to set the MINIDUMP_SAVE_PATH (https://searchfox.org/mozilla-central/rev/a1592902acabf9bded973067133baaac1457f3d3/testing/mozbase/mozcrash/mozcrash/mozcrash.py#201) to a directory whose files are uploaded as artifacts (just like https://searchfox.org/mozilla-central/rev/a1592902acabf9bded973067133baaac1457f3d3/testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py#270).

Comment 1

[Mike Hommey [:glandium]]

I'd argue the xpcshell selftests should run at the very start of the xpcshell test task.

Comment 2

[(Away)]

+1 for making these part of the xpcshell job. It's a pain when failures turn the build job red and prevent getting results on any downstream test jobs.

Comment 3

[Geoff Brown [:gbrown]]

I may not get around to this for a while...don't mind if someone else wants to work on it!

Comment 4

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b2a5a0f498f3cc426c684a86cafbf56e7734931

Comment 5

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d68947b74f57b8b87440ebd9d0431713562ede67

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Geoff, are you planning to finish this? thanks!

Comment 7

[Geoff Brown [:gbrown]]

Yes, but comment 3 still applies.

Comment 8

[Geoff Brown [:gbrown]]

Bug 1651829 (and others blocking bug 1210759) are progressing...that might be helpful here.

Comment 9

[Mike Hommey [:glandium]]

Stealing this. I have a patch 95% the way there.

I'm not sure the move on its own will solve the original problem. Marco, do you have an example of a crash that was a problem to debug for lack of minidump?

Comment 11

[Marco Castelluccio [:marco]]

(In reply to Mike Hommey [:glandium] from comment #9)

Stealing this. I have a patch 95% the way there.

I'm not sure the move on its own will solve the original problem. Marco, do you have an example of a crash that was a problem to debug for lack of minidump?

It was a crash in a push by Calixte with some ccov-related changes. Given the creation date of this bug and the dates of Calixte's try pushes, probably one of e47a77ccee096cf2b442a22ed4d0401dd347ea78, 591790aedfcacae6b421c2e9c59f5323e342dca9 or 12bd727ae74a5b5c28dc03e284c362359442d580.

Comment 12

[Mike Hommey [:glandium]]

Attachment: Bug 1614626 - Move xpcshell selftests to before running actual xpcshell tests.

This allows to test them in the exact same environment as the tests are
going to run, which turns out to have revealed a few issues that would
only appear once xpcshell tests fail, impeding on debugging those
failures.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/929e1acc42af
Move xpcshell selftests to before running actual xpcshell tests. r=gbrown

Comment 14

[Dorel Luca [:dluca]]

Backed out changeset 929e1acc42af (bug 1614626) for XPCshell failures in extensions/test/xpcshell/test_ext_dns.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311353092&repo=autoland&lineNumber=1972

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=929e1acc42af5428c14a2b6b56b3fce5bdc2d23e

Backout:
https://hg.mozilla.org/integration/autoland/rev/3a40fe8b0122cf9dff947e2777da2371e663eb68

Comment 15

[Atila Butkovits]

It also caused valgrid bustages: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Csuperseded%2Cretry%2Cusercancel&tochange=0067bb1fb8e45078f4383c9e371dd1c7eafbffbe&fromchange=b33d7cff301d89fa988c7447ef059685347690fc&searchStr=valgrind&selectedTaskRun=E4pEC3g7T7uS4vu3nIMeRg.0

Comment 16

[Geoff Brown [:gbrown]]

test_ext_dns.js is normally skipped in those tasks...is there something wrong with the mozinfo changes?

Comment 17

[Mike Hommey [:glandium]]

There is no mozinfo change in the patch that would explain that. The problem is that running the self tests does change mozinfo, most notably wrt prefs.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e099635be739
Move xpcshell selftests to before running actual xpcshell tests. r=gbrown

Comment 19

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e099635be739

Comment 20

[Mitchell Hentges [:mhentges] 🦀]

Comment on attachment 9165828 [details]
Bug 1614626 - Move xpcshell selftests to before running actual xpcshell tests.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed to uplift https://bugzilla.mozilla.org/show_bug.cgi?id=1654457, otherwise python-tests running as part of make fail, because they're run with the wrong virtualenv.
• User impact if declined: None
• Fix Landed on Version: 81
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a build-only patch
• String or UUID changes made by this patch:

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9165828 [details]
Bug 1614626 - Move xpcshell selftests to before running actual xpcshell tests.

Needed to better support running mach on newer macOS releases. Approved for 78.8esr.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/a00af3bc78a8