421720 - Add ability to run unit and mochi tests that should fire an assertion

Status: NEW

(Testing :: General, defect)

Description

[Andrew Schultz]

It's sometimes useful to have a test that would cause an assertion and still be considered a PASS (abuse a class make sure it doesn't do something bad (like crash)).  bug 358599 is one example.

Unit tests are run with fatal assertions http://mxr.mozilla.org/seamonkey/source/tools/test-harness/xpcshell-simple/test_all.sh#47

bug 404077 would make assertions fatal in the mochitest and having a test in the standard mochitest suite that has a decent chance of crashing on failure is not good behavior.

crashtests serve this purpose for reftest-able tests (so long as assertions are fatal there too), but tests needing more privs don't have a home.

Comment 1

[Andrew Schultz]

> so long as assertions are fatal there too

*not* fatal

Comment 2

[Jeff Walden [:Waldo]]

Why should assertions not be fatal?  If the condition is not a fatal error, it should not be an assert.  If it's merely something that's not yet fixed to not assert, don't worry about adding a Mochitest that asserts.  My suspicion is that someday we'll split our Mochitests into ones that assert and ones that don't, and we'll run half with fatal assertions and half without until the half without becomes none without.  One more test that asserts, in that case, will not be a problem.

I fail to see what's wrong with a Mochitest which, if it fails, crashes.  You'd like to know what the other tests do in such a build, but I see this as yet more motivation to make sure fixes are correct.

We could allow reftests to have privileges by simply running them with the HTTP annotation and granting privileges to the server on which the HTTP tests run, as an awful short-term hack.  However, I still don't see why Mochitests are insufficient for doing this, and I doubt this would be a short-term, low-use hack anyway.

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Yeah, I've crashed mochi a fair number of times fumbling around the JS engine.  It would be _kinda_ cool if runtests.py could tell you the test it was running when it crashed, without needing to use non-default log options and search backwards through the log file, but attaching gdb isn't too hard, and the crashes are eminently reproducible. :)

Comment 4

[Jesse Ruderman]

I think Andrew is asking for something more specific: he has some JavaScript code that requires privilege and uses some APIs incorrectly.  If the same API abuse occurred in Firefox, it would be a bug, so asserting makes sense.

Comment 5

[Jeff Walden [:Waldo]]

That should throw an exception, but it shouldn't assert, in my opinion.

Comment 6

[Jesse Ruderman]

Why?  Just because the API is scriptable?

Comment 7

[Martijn Wargers (dead)]

Yes, afaik, the idea is that the scriptable api should never crash. If assertions would basically mean the same as crashing, then the api should never assert either.

Comment 8

[Benjamin Smedberg]

I believe that scriptable APIs should be allowed to crash, at least in performance-sensitive code. So if you use the API incorrectly, we should assert+crash.

On the other hand, if adding parameter validation is not a performance bottlenec, I believe we should do it (and make the methods throw exceptions, not assert or crash), so that extension authors are less-able to crash the browser by accident.

Comment 9

[(not currently active) Ted Mielczarek]

Everything left in Core:Testing is going to Testing:General. Filter on CleanOutCoreTesting to ignore.