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)
Tracking
(firefox34 fixed, firefox35 fixed)
RESOLVED
FIXED
mozilla35
People
(Reporter: sydpolk, Assigned: spohl)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
spohl
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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
Reporter | ||
Comment 2•10 years ago
|
||
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.
Reporter | ||
Comment 3•10 years ago
|
||
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$
Assignee | ||
Comment 4•10 years ago
|
||
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.
Assignee | ||
Comment 5•10 years ago
|
||
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
Assignee | ||
Comment 7•10 years ago
|
||
This also fixes the mochitest harness when run locally via mach.
Attachment #8481890 -
Attachment is obsolete: true
Assignee | ||
Comment 8•10 years ago
|
||
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)
Assignee | ||
Comment 9•10 years ago
|
||
Comment 10•10 years ago
|
||
(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 11•10 years ago
|
||
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)
Assignee | ||
Comment 12•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
Attachment #8495974 -
Flags: review?(ted) → review?(jmaher)
Updated•10 years ago
|
Attachment #8495974 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 13•10 years ago
|
||
Updated commit message with r=jmaher. Carrying over r+.
Attachment #8495974 -
Attachment is obsolete: true
Attachment #8496041 -
Flags: review+
Comment 14•10 years ago
|
||
Backed out old patch and relanded on oak
https://hg.mozilla.org/projects/oak/rev/dbf4069f434a
Comment 15•10 years ago
|
||
Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/427a562e16d4
Comment 16•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 17•10 years ago
|
||
Landed on aurora in the Mac V2 signing combined patch in bug 1047584
status-firefox34:
--- → fixed
status-firefox35:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•