Open Bug 977240 Opened 6 years ago Updated 6 years ago

Run valgrind tests on test slaves

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: jgriffin, Unassigned)

References

(Blocks 1 open bug)

Details

Currently valgrind tests are run on linux64 opt builds on build slaves; it was proposed on dev.platform that we split these up into test suites and run them on test slaves.

https://groups.google.com/forum/#!topic/mozilla.dev.platform/rBr0Ob0AODc
Whiteboard: [MemShrink]
Currently browser/config/mozconfigs/linux64/valgrind looks like this:

> no_tooltool=1
> 
> . $topsrcdir/browser/config/mozconfigs/linux64/nightly
> 
> ac_add_options --enable-valgrind
> ac_add_options --disable-jemalloc
> ac_add_options --disable-elf-hack
> ac_add_options --enable-optimize="-g -O -freorder-blocks"
> ac_add_options --disable-install-strip
> 
> # Include the override mozconfig again (even though the above includes it)
> # since it's supposed to override everything.
> . "$topsrcdir/build/mozconfig.common.override"

And browser/config/mozconfigs/linux32/valgrind is identical except that
|linux64| is replaced with |linux32|.

It would be nice if we didn't have to do a distinct build for Valgrind, but
given the above, that sounds hard to achieve. (Nb: bug 977067 is about removing
the --disable-jemalloc part, but even then significant differences remain.)
Looks like step 1 is to cause our valgrind builds to be uploaded, so we can use them from test slaves; then we'll need a mozharness script for running the valgrind tests.
Depends on: 977351
I think we should just use normal builds and run valgrind on them.
> I think we should just use normal builds and run valgrind on them.

Doesn't the --disable-install-strip requirement prevent this?
Ah shoot, we don't have the debugging symbols for non nightlies. Valgrind doesn't need them all does it?
I think Valgrind does need debugging info in order to show filenames and line numbers in its error messages.
But it doesn't need all sections for that, just a few. And it should be able to read the debug info from separate files. So we could package those instead of doing entirely new builds. Arguably, AIUI, we currently don't upload the full symbols for disk space, but if we're uploading them as part of the valgrind builds, we wouldn't be adding weight.
Well, I think it's a fine idea if it can be made to work :)
What's going to happen to the existing valgrind tests that happen as part of the build?

If we remove 'mach valgrind-test' from this build, then I propose we turn valgrind builds into another flavour of our general linux builds, much like ASan builds. Currently the valgrind builds are implemented in very much a different way than other builds.
(In reply to Chris AtLee [:catlee] from comment #9)
> What's going to happen to the existing valgrind tests that happen as part of
> the build?
> 
> If we remove 'mach valgrind-test' from this build, then I propose we turn
> valgrind builds into another flavour of our general linux builds, much like
> ASan builds. Currently the valgrind builds are implemented in very much a
> different way than other builds.

That's the intention; we'd move the execution of valgrind tests to a test job run on test slaves.
Whiteboard: [MemShrink]
You need to log in before you can comment on or make changes to this bug.