Fail a suite if onTest or onCleanup throws an exception. Fixes valueo…

…fGH-5.

hiro.js

@@ -135,22 +135,19 @@ var hiro = (function (window, undefined) {
 
 			if (self.methods.onTest) {
 				self.onTest_ = function (test) {
-					if (test.suite.name == self.name)
+					if (test.suite.name != self.name)
+						return;
+
+					try {
 						self.methods.onTest.apply(test);
+					} catch (exc) {
+						test.fail_({ assertion: 'onTest', result: exc.toString() });
+					}
 				};
 
 				hiro.bind('test.onStart', self.onTest_);
 			}
 
-			if (self.methods.onCleanup) {
-				self.onCleanup_ = function (test) {
-					if (test.suite.name == self.name)
-						self.methods.onCleanup.apply(test);
-				};
-
-				hiro.bind('test.onComplete', self.onCleanup_);
-			};
-
 			if (self.methods.waitFor) {
 				self.status = 'waiting';
 				self.snapshot = timestamp();
@@ -192,10 +189,6 @@ var hiro = (function (window, undefined) {
 					hiro.unbind('test.onStart', this.onTest_);
 				}
 
-				if (this.methods.onCleanup) {
-					hiro.unbind('test.onCleanup', this.onCleanup_);
-				}
-
 				this.status = 'done';
 			}
 
@@ -312,6 +305,14 @@ var hiro = (function (window, undefined) {
 				return false;
 			}
 
+			if (this.suite.methods.onCleanup) {
+				try {
+					this.suite.methods.onCleanup.apply(this);
+				} catch (exc) {
+					this.fail_({ assertion: 'onComplete', result: exc.toString() });
+				}
+			}
+
 			if (this.failed) {
 				hiro.trigger('test.onComplete', [ this, false ]);
 				return false;
@@ -349,9 +350,11 @@ var hiro = (function (window, undefined) {
 		run: function () {
 			hiro.trigger('test.onStart', [ this ]);
 
-			this.status = 'running';
-			this.func.apply(this, this.args);
-			this.snapshot = timestamp();
+			if (!this.failed) {
+				this.status = 'running';
+				this.func.apply(this, this.args);
+				this.snapshot = timestamp();
+			}
 
 			if (!this.paused)
 				this.status = 'done';
@@ -445,7 +448,8 @@ var hiro = (function (window, undefined) {
 		internals_: {
 			Suite:    Suite,
 			Test:     Test,
-			getSuite: getSuite
+			getSuite: getSuite,
+			events:   events
 		},
 
 		changeTimeout: function (timeout) {
@@ -487,8 +491,13 @@ var hiro = (function (window, undefined) {
 			if (events[name] === undefined)
 				return;
 
+			if (!handler) {
+				events[name] = [];
+				return;
+			}
+
 			for (var i = 0, h; h = events[name][i]; i++) {
-				if (h == handler) {
+				if (h === handler) {
 					events[name].splice(i, 1);
 					return;
 				}

test.js

@@ -157,6 +157,12 @@ hiro.module('TestRunnerTests', {
 		this.args = [ hiro_, hiro_.internals_.Test, hiro_.internals_.Suite ];
 	},
 
+	onCleanup: function () {
+		var hiro_ = this.window.hiro;
+		hiro_.unbind('test.onStart');
+		hiro_.unbind('test.onCleanup');
+	},
+
 	testRun: function (hiro_, Test, Suite) {
 		var that = this;
 
@@ -352,7 +358,6 @@ hiro.module('TestRunnerTests', {
 		};
 
 		suite.setUp_();
-
 		hiro_.changeTimeout(500);
 		this.expect(15);
 
@@ -372,6 +377,75 @@ hiro.module('TestRunnerTests', {
 		this.assertFalse(test.failed);
 		this.assertFalse(test.paused);
 		this.assertTrue(test.complete_());
+	},
+
+	testExceptionInsideOnTest: function (hiro_, Test, Suite) {
+		var that = this;
+		var suite = new Suite('test', {});
+		var test;
+
+		function testCase() {
+			// Test case shouldn't be called if there's an error in the onTest method
+			that.assertTrue(false);
+		}
+
+		test = new Test('testDummy', testCase, suite);
+
+		suite.methods.onTest = function () {
+			throw new Error("Wub wub wub");
+		};
+
+		suite.setUp_();
+		hiro_.changeTimeout(500);
+		this.expect(9);
+
+		this.assertEqual(test.name, 'testDummy');
+		this.assertEqual(test.status, 'ready');
+		this.assertFalse(test.failed);
+		this.assertFalse(test.paused);
+		this.assertTrue(test.snapshot == null);
+
+		test.run();
+		this.assertEqual(test.status, 'done');
+		this.assertTrue(test.failed);
+		this.assertFalse(test.paused);
+		this.assertFalse(test.complete_());
+	},
+
+	testExceptionInsideOnCleanup: function (hiro_, Test, Suite) {
+		var that = this;
+		var suite = new Suite('test', {});
+		var test;
+
+		function testCase() {
+			that.assertTrue(true);
+		}
+
+		test = new Test('testDummy', testCase, suite);
+
+		suite.methods.onCleanup = function () {
+			throw new Error("Wub wub wub");
+		};
+
+		suite.setUp_();
+		hiro_.changeTimeout(500);
+		this.expect(11);
+
+		this.assertEqual(test.name, 'testDummy');
+		this.assertEqual(test.status, 'ready');
+		this.assertFalse(test.failed);
+		this.assertFalse(test.paused);
+		this.assertTrue(test.snapshot == null);
+
+		test.run();
+		this.assertEqual(test.status, 'done');
+		this.assertFalse(test.failed);
+		this.assertFalse(test.paused);
+		this.assertFalse(test.complete_());
+
+		// Test.complete_ changes test status to 'failed'
+		// if there's an exception inside of onCleanup
+		this.assertTrue(test.failed);
 	}
 });