Bug 521922's patch (Patch for running xpcshell tests cleanly in electrolysis in either content or chrome processes) does a great job, but there's a problem. When the child test script generates a "SyntaxError:" error output, runxpcshelltests.py doesn't detect it and hence, consider the test as passed. This is not appropriate as we really should fail here.

I investigated this, and the reason is quite simple. in head.js::_execute_test(), run_test *DOES NOT* throw when run_test has a syntax error. It throws for a lot of cases, e.g. reference errors, but not for syntax... I'm not sure why. Hence, it doesn't output TEST-UNEXPECTED-FAIL, which, in turn, goes unnoticed in xpcshelltests.py. the test passes, but shouldn't. What could be used for syntax error detections by xpcshelltests.py is the "SyntaxError:" keyword that writes to the child output. That's what I'm doing in this patch. Now, only question is : There might be some tests that output "SyntaxError:" on purpose, but that seems *really* unlikely to me. What should we do with that?

take a look at this one instead, thanks.

Comment on attachment 427476 [details] [diff] [review] patch v.2 This looks good. Thanks for figuring this out. Let's change the regex to search for ": SyntaxError: ": it looks like we always get the leading colon and space, and that ought to make it nearly impossible that some random test output would match.

updated patch per review comment.

Pushed only to /projects/electrolysis because this is not an m-c issue http://hg.mozilla.org/projects/electrolysis/rev/0b53086e8a2f3693c929f78aefba224bf2eef888

Just FWIW, jduell isn't really a peer on the test harness code, so I'd appreciate it if you did get peer review on patches to the test harnesses. This is a pretty minor change, and it's only in e10s, so this particular case is not a big deal.

(In reply to comment #7) > Just FWIW, jduell isn't really a peer on the test harness code, so I'd > appreciate it if you did get peer review on patches to the test harnesses. This > is a pretty minor change, and it's only in e10s, so this particular case is not > a big deal. k, I'll do next time, for sure. thanks.