990354 - xpcshell should fail tests on uncaught exception

Status: NEW

(Testing :: XPCShell Harness, defect)

Description

[Boris Zbarsky [:bzbarsky]]

We do this in our other harnesses (e.g. mochitest).  Needing to jump through the hoops described in http://gregoryszorc.com/blog/2014/03/30/how-promises-and-tasks-are-improving-tests/ is pretty unfortunate.

We may need to have a way of annotating tests as exempt from the new setup temporarily until we can fix all our existing tests to work with it.  Or something.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Boris, if you can describe the exact behavior you are expecting, I am willing to mentor this bug.

Comment 2

[Boris Zbarsky [:bzbarsky]]

The behavior in, say, mochitest is that any uncaught exception is treated as a test failure and immediately terminates the test, and the test failure message has the exception details.  This is particularly important for sync tests that don't manually call SimpleTest.finish(), because for those tests an exception would mean that part of the test didn't get run but the test harness thinks the test finished successfully.  But it's useful for tests that do waitForExplicitFinish() as well: it converts a test timeout due to an exception (which prevented us ever reaching the finish() call) into a failure that shows the exception and is much simpler to debug.

So I'd like us to do the same thing in xpcshell tests.  Instead of an exception in an async test causing a hard-to-debug test timeout, it would be better if it immediately failed the test, with information about the exception.

Does that make sense?

Comment 3

[Saurabh Anand [:sawrubh]]

Do you still want to mentor this bug?

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Boris, do I understand correctly that you are talking specifically of tests that do not use `add_task`? Because `add_task`-based tests, in addition to being generally much more readable, do not timeout in case of uncaught exception.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Saurabh, indeed

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yes, I'm talking about tests that do do_test_pending/do_test_finished.

Do add_task based tests fail immediately on uncaught exception in the generator?

Comment 7

[David Teller [:Yoric] - still alive but not very active]

(In reply to Boris Zbarsky [:bz] from comment #6)
> Do add_task based tests fail immediately on uncaught exception in the
> generator?

If I recall correctly, they run the cleanup functions first.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, that's fine, as long as they trigger a test failure.  ;)

Comment 9

[Gregory Szorc [:gps]]

We have explicit test coverage that add_task fails in certain scenarios:

https://hg.mozilla.org/mozilla-central/file/2d88803a0b9c/testing/xpcshell/selftest.py#l577
https://hg.mozilla.org/mozilla-central/file/2d88803a0b9c/testing/xpcshell/selftest.py#l104
https://hg.mozilla.org/mozilla-central/file/2d88803a0b9c/testing/xpcshell/selftest.py#l114

Whoever reviews this will likely insist on selftest.py being updated.