957410 - |mach valgrind-test| doesn't detect failures in child processes

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Nicholas Nethercote [inactive]]

|mach valgrind-test| currently uses Valgrind's --error-exitcode=1 flag to detect if Valgrind finds any errors, in which case it returns an appropriate exit code.

However, this only works if Valgrind finds errors in the initial process. Errors in child processes aren't detected. This isn't such a big deal now, but will be in the future with e10s.

The only solution I can see is to scan Valgrind's output, looking for the presence of "ERROR SUMMARY" lines with non-zero error counts. This is what TBPL does
(https://hg.mozilla.org/webtools/tbpl/file/tip/php/inc/GeneralErrorFilter.php#l49) in order to highlight these lines in TBPL output. But in order to turn the jobs red we need a non-zero exit code, so I think |mach valgrind-test| needs to do likewise.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Make |mach valgrind-test| detect errors in child processes.

Here's an attempt that doesn't work. I cargo-culted the test_output/handler
stuff from testing/mochitest/mach_commands.py, but I don't understand it at all
and it doesn't work -- NonZeroErrorSummaryFilter.filter doesn't seem to get
called at all.

gps, what is the right way to do this?

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8356904 [details] [diff] [review]
Make |mach valgrind-test| detect errors in child processes.

Review of attachment 8356904 [details] [diff] [review]:
-----------------------------------------------------------------

mach's logging internals are crap and have been in need of a refactor.

This likely isn't working because the "unstructured" (classical) logging a) isn't being converted to structured logging or b) that structured logger's output isn't being captured.

b can be solved by calling log_manager.register_structured_logger('name.of.logger'). I have no clue what logger is being used behind the scenes here. But if you register the root logger (logging.getLogger()), it should catch it.

Comment 3

[Nicholas Nethercote [inactive]]

Ok, the mochitest code was a red herring. The mochitests produced logging output, but Valgrind just writes non-logging output to stderr, so they're not the same thing.

To summarize the situation and what I want...

- Valgrind's output currently goes to stderr, i.e. it's printed to the terminal.

- I want to still print it to stderr, but I also want to parse the output in order to pick out some interesting bits.

- So I probably need to capture the output in some fashion -- in a buffer, or a file?

- Valgrind has options that allow its output to be sent to a file or an fd (or even a socket) instead of stderr.

- Ideally the printing of the output would happen in real-time, rather than being done in a single hit at the end -- this is useful for people running it locally.

Comment 4

[Nicholas Nethercote [inactive]]

> - Ideally the printing of the output would happen in real-time, rather than
> being done in a single hit at the end -- this is useful for people running
> it locally.

If I'm willing to give up this requirement, it's not hard -- I can just use Valgrind's --log-file option and then print and parse the output at the end. Hmm.

Comment 5

[Nicholas Nethercote [inactive]]

Maybe I can use --log-fd and set up a tee-like fd that both prints the output and captures it into a buffer/file?

Comment 6

[(not currently active) Ted Mielczarek]

mozrunner redirects stderr to stdout, so you shouldn't have to jump through hoops here. You probably just want to do something similar to what Mochitest is doing:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#883

outputHandler is just a callable, so you can pass a function there, Mochitest is using a class with a __call__ method so it can store some extra stuff, but a closure is probably sufficient.

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#1114

That callback should get called for every line of stderr/stdout output.

Comment 7

[Nicholas Nethercote [inactive]]

Thanks, Ted! That works nicely.

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: Make |mach valgrind-test| detect errors in child processes.

Rather than relying entirely on the exit code to determine the test's outcome,
we now look at the output to determine if errors occurred, and look at the exit
code only to determine if something when wrong (such as Valgrind crashing).

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad741e8ab1b7

Comment 10

[Carsten Book [:Tomcat]]

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