Open Bug 990354 Opened 7 years ago Updated 2 years ago
xpcshell should fail tests on uncaught exception
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.
Boris, if you can describe the exact behavior you are expecting, I am willing to mentor this bug.
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?
Do you still want to mentor this bug?
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.
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?
(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.
Oh, that's fine, as long as they trigger a test failure. ;)
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.
You need to log in before you can comment on or make changes to this bug.