Comment 4 for bug 243704

(In reply to comment #3)
> (From update of attachment 326043 [details])
> >+ LOG(("Running Test: %s %d\n", testCommand.get(), rv));
>
> 'rv' should always be success here, right? Why log it?

That is a shameful piece left from debugging...

> Put single quotes around the %s in the string to make it easy to tell where it
> starts/ends?
>
> >+ if (process->Run(PR_TRUE, args, 2, NULL)) {
>
> Uh... have you tested this? Using what tests?
>
> Is passing null for the out param even legal? I wouldn't have thought so,
> given typical IDL contracts...

Actually, the function does nothing with the argument... it doesn't return a pid. But I agree it doesn't hurt to replace it.

> Also, does Run() really throw if the process
> exits with a nonzero error code? Or should you be looking at
> process->GetExitValue()?

Looking more closely to the code, you are right. I shouldn't write such patches late in the night.