Closed Bug 1060652 Opened 10 years ago Closed 10 years ago

In the OAK build, running mochitest using the .dmg file and tests does not work because of missing dylibs

Categories

(Testing :: Mochitest, defect)

x86
macOS
defect
Not set
normal

Tracking

(firefox34 fixed, firefox35 fixed)

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: sydpolk, Assigned: spohl)

References

Details

Attachments

(1 file, 3 obsolete files)

1. Download firefox-firefox-33.0a1.en-US.mac.dmg and firefox-33.0a1.en-US.mac.tests.zip. 2. Mount the dmg. 3. Copy /Volumes/Nightly/FirefoxNightly.app to your drive. I copied it to ~/firefoxes. 4. mkdir tests 5. unzip firefox-33.0a1.en-US.mac.tests.zip 6. cd .. 7. Run the setup instructions in https://etherpad.mozilla.org/running-out-of-tests-zip (but don't run the runtests.py script yet) 8. From your vitualenv, run the following: (mochitest)Mountain-Lion-Xcode-6:~ jazzman$ python tests/mochitest/runtests.py --total-chunks 5 --this-chunk 1 --appname=~/firefoxes/FirefoxNightly.app/Contents/MacOS/firefox --utility-path=tests/bin --extra-profile-file=tests/bin/plugins --certificate-path=tests/certs --autorun --close-when-done --console-level=INFO --quiet --log-file=raw.log --chunk-by-dir=4 > /tmp/log You will get spew, but the last of the log says: dyld: Library not loaded: @executable_path/libnss3.dylib Referenced from: /Users/jazzman/tests/bin/certutil Reason: image not found TEST-UNEXPECTED-FAIL | runtests.py | Certificate integration failed (END) To workaround, do: export DYLD_LIBRARY_PATH=~/firefoxes/FirefoxNightly.app/Contents/MacOS Run the command again. The tests proceed.
Attached patch Patch (obsolete) (deleted) — Splinter Review
Thanks, Syd! All your work helped me come up with this patch. I kicked off a test run on Oak, which should tell us how we're looking. More work may still be necessary, but this should fix the immediate issue with the library load path: https://tbpl.mozilla.org/?tree=Oak&rev=c91ee24525b9
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
I actually don't agree with the approach of this patch. The necessary dylibs are included in the tests directory, where the xpcshell binaries are. If you have DYLD_LIBRARY_PATH there, then you can run these tests on future versions of Firefox if those versions change dylibs in the future.
However, those dylibs are scattered all of the place in the tests directory. Somebody should clean this up: sydpolkzillambp:tests spolk$ find . -name \*.dylib ./bin/plugins/JavaTest.plugin/Contents/MacOS/libnptestjava.dylib ./bin/plugins/SecondTest.plugin/Contents/MacOS/libnpsecondtest.dylib ./bin/plugins/Test.plugin/Contents/MacOS/libnptest.dylib ./mochitest/chrome/toolkit/components/ctypes/tests/chrome/libjsctypes-test.dylib ./xpcshell/tests/js/xpconnect/tests/components/native/libxpctest.dylib ./xpcshell/tests/modules/libmar/tests/unit/libnss3.dylib ./xpcshell/tests/toolkit/components/ctypes/tests/unit/libjsctypes-test.dylib ./xpcshell/tests/toolkit/crashreporter/test/unit/libtestcrasher.dylib ./xpcshell/tests/toolkit/crashreporter/test/unit_ipc/libtestcrasher.dylib ./xpcshell/tests/xpcom/tests/unit/libtest656331.dylib ./xpcshell/tests/xpcom/tests/unit/libtestcompnoaslr.dylib ./xpcshell/tests/xpcom/tests/unit/libtestcomponent.dylib sydpolkzillambp:tests spolk$
The xpcshell binary is copied into the .app bundle before it's executed. So, this patch actually points the library load path to the dylibs sitting next to the xpcshell binary, which is most likely what we want and prefer.
Comment on attachment 8481890 [details] [diff] [review] Patch This turned out to be all we needed for mochitest. The remaining failures were due to a mistake in the patch for bug 1055774. Mochitest is now green: https://tbpl.mozilla.org/?tree=Oak&rev=797d19721fee
Attachment #8481890 - Attachment description: Patch (wip) → Patch
Attached patch Patch (obsolete) (deleted) — Splinter Review
This also fixes the mochitest harness when run locally via mach.
Attachment #8481890 - Attachment is obsolete: true
Comment on attachment 8486544 [details] [diff] [review] Patch Try push is almost completely green, so requesting review (updater xpcshell test failures and gaia python integration test suite failures are handled in separate bugs): https://tbpl.mozilla.org/?tree=Try&rev=7362867ad903
Attachment #8486544 - Flags: review?(ted)
(In reply to Syd Polk :sydpolk from comment #3) > However, those dylibs are scattered all of the place in the tests directory. > Somebody should clean this up: This isn't a thing to be cleaned up, FWIW, these are individual tests that use shared libraries as part of the test. (Or for the first few entries there, the test plugins we use.)
Comment on attachment 8486544 [details] [diff] [review] Patch Review of attachment 8486544 [details] [diff] [review]: ----------------------------------------------------------------- I'm a little worried about scattering these path manipulations all over the tree. I think that's going to make it very hard to keep things from breaking in the future. I'm not r+ing this because I have a few issues with it, but I don't think there's anything insurmountable. ::: browser/app/macbuild/Contents/MacOS-files.in @@ +4,3 @@ > /firefox-bin > +/pk12util > +/ssltunnel This is OK as a short-term fix, but the harness already supports running these files from a different directory, so it feels like we should just be using that instead. If you just made the mach command point its utilityPath at dist/bin instead of the MacOS dir, would that work? ::: testing/mochitest/runtests.py @@ +293,5 @@ > > # get testing environment > env = environment(xrePath=self._xrePath) > env["XPCOM_DEBUG_BREAK"] = "warn" > + env["DYLD_LIBRARY_PATH"] = os.path.join(os.path.dirname(self._xrePath), 'MacOS') I think it'd make sense to push this down into the environment function. (Looks like you could drag the LD_LIBRARY_PATH bit with it, since all the callers seem to set that as well.) That would at least make it so you only set it once. @@ +677,5 @@ > # on the command line to select a particular version of httpd.js. If not > # specified, try to select the one from hostutils.zip, as required in bug 882932. > if not options.httpdPath: > options.httpdPath = os.path.join(options.utilityPath, "components") > + if not os.path.exists(options.httpdPath) and (sys.platform == 'osx' or sys.platform == "darwin"): Use mozinfo.os == 'mac' here, please. Do we really hit this case in practice? This should only happen in packaged builds, and httpd.js should wind up in the test package's components dir because of the packaging bits here: http://hg.mozilla.org/mozilla-central/annotate/5e704397529b/testing/xpcshell/Makefile.in#l48
Attachment #8486544 - Flags: review?(ted)
Attached patch Patch (obsolete) (deleted) — Splinter Review
Mochitests are turning green on try with this patch: https://tbpl.mozilla.org/?tree=Try&rev=55e45bb6a000 (In reply to Ted Mielczarek [:ted.mielczarek] from comment #11) > ::: browser/app/macbuild/Contents/MacOS-files.in > @@ +4,3 @@ > > /firefox-bin > > +/pk12util > > +/ssltunnel > > This is OK as a short-term fix, but the harness already supports running > these files from a different directory, so it feels like we should just be > using that instead. If you just made the mach command point its utilityPath > at dist/bin instead of the MacOS dir, would that work? Discussed this on IRC and it's my understanding that this is okay now, since these files won't be packaged into the final bundle. > ::: testing/mochitest/runtests.py > @@ +293,5 @@ > > > > # get testing environment > > env = environment(xrePath=self._xrePath) > > env["XPCOM_DEBUG_BREAK"] = "warn" > > + env["DYLD_LIBRARY_PATH"] = os.path.join(os.path.dirname(self._xrePath), 'MacOS') > > I think it'd make sense to push this down into the environment function. > (Looks like you could drag the LD_LIBRARY_PATH bit with it, since all the > callers seem to set that as well.) That would at least make it so you only > set it once. Moved this to automationutils.py. Interestingly, automationutils.py should already do the right thing for LD_LIBRARY_PATH, so I'm not sure why this is set here too. Would need to investigate before moving LD_LIBRARY_PATH, but this shouldn't stop this patch. > @@ +677,5 @@ > > # on the command line to select a particular version of httpd.js. If not > > # specified, try to select the one from hostutils.zip, as required in bug 882932. > > if not options.httpdPath: > > options.httpdPath = os.path.join(options.utilityPath, "components") > > + if not os.path.exists(options.httpdPath) and (sys.platform == 'osx' or sys.platform == "darwin"): > > Use mozinfo.os == 'mac' here, please. Is mozinfo.isMac okay? > Do we really hit this case in > practice? This should only happen in packaged builds, and httpd.js should > wind up in the test package's components dir because of the packaging bits > here: > http://hg.mozilla.org/mozilla-central/annotate/5e704397529b/testing/xpcshell/ > Makefile.in#l48 You're right. Removed.
Attachment #8486544 - Attachment is obsolete: true
Attachment #8495974 - Flags: review?(ted)
Attachment #8495974 - Flags: review?(ted) → review?(jmaher)
Attachment #8495974 - Flags: review?(jmaher) → review+
Attached patch Patch (deleted) — Splinter Review
Updated commit message with r=jmaher. Carrying over r+.
Attachment #8495974 - Attachment is obsolete: true
Attachment #8496041 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Landed on aurora in the Mac V2 signing combined patch in bug 1047584
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: