Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Unit tests pass for certain uncaught exceptions #2594

Open
peterflynn opened this issue Jan 17, 2013 · 4 comments
Open

Unit tests pass for certain uncaught exceptions #2594

peterflynn opened this issue Jan 17, 2013 · 4 comments
Assignees

Comments

@peterflynn
Copy link
Member

For example bug #2568 -- which should have failed a test by throwing an exception but didn't because dispatchEvent() hides thrown exceptions from its callers. (And the exception didn't change anything that the test was checking for, which granted means the bug wasn't that bad... but it would be nice to fail the test anyway).

NJ pointed out that many tests that use dispatchEvent() could just use $.trigger() instead. But there are a few tests that interact with non-jQuery listeners (e.g. Bootstrap or CodeMirror code) where we're always stuck with dispatchEvent(). Are there enough of those to care?

There's also the question of uncaught exceptions in async code. Normally I'd expect that to mean the async code never finishes and thus the Jasmine testcase fails by virtue of a timeout... but is that always true?

If we wanted to be fancy we could use window.onerror to guarantee noticing any uncaught exception. But it may not be worth the effort (though in the main app a similar approach could be useful for error reporting).

@peterflynn
Copy link
Member Author

NJ points out that trigger() won't work for capture listeners either (since jQuery doesn't support them... grumble grumble).

@njx
Copy link
Contributor

njx commented Jan 17, 2013

It won't work for any addEventListener() listeners in general (not just capture listeners), I think. (Though it does work for non-jQuery listeners attached via the DOM onxxx attributes.)

@peterflynn
Copy link
Member Author

Right, it's just that our own code (should) only use addEventListener() when we need a capture listener. For normal listeners we should always be going through jQuery.

@ghost ghost assigned dangoor Jan 18, 2013
@gruehle
Copy link
Member

gruehle commented Jan 18, 2013

Reviewed, assigned to @dangoor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants