Closed Bug 998277 Opened 6 years ago Closed 6 years ago
Uncaught async Promise errors should cause sdk tests to fail
No description provided.
Yoric, can you give example tests where this is actually a concern? most (if not all) async tests already have something like somePromie().then(done); which to my understanding already fails in the case of async promise error, though not explicitly, but because of a test timeout.
Well, that's assuming that you have added a terminator to all promises. With xpcshell tests and mochitests we found out that we are missing a number of errors simply because ensuring that we have caught all async errors is hard/impossible. Promise.jsm provides a way to detect most of these errors, but if the test suite doesn't take this into account, that's not very useful.
so, we are usually careful about stuff like that, but i understand this is more of a "defense in depth" thing.. then i guess it depends on bug 881047.
Depends on: 881047
Priority: -- → P2
Test with the patch in bug 1016387
Patch in bug 1016387 does not work for SDK tests, we'll have to hook into Promise.jsm's `Debugger.addUncaughtErrorObserver` into the SDK test harness.
Attachment #8471857 - Flags: review?(evold) → review+
Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/fc3d108c8ca86b213f95fb5cc3dc7a4f06239cb7 Bug 998277 - Enable tests breaking on uncaught promise rejections https://github.com/mozilla/addon-sdk/commit/91c8e46257a43296620f1a5bab328687c73515d0 Merge pull request #1582 from jsantell/998277 Bug 998277 - Enable tests breaking on uncaught promise rejections, r=@erikvold
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.