Skip to content

Commit 2cb9c23

Browse files
edusperoniedusperoni
committed
test: keep worker errors and failure paths version-agnostic across Jasmine releases
Jasmine 3+ installs a global error handler that fails whichever spec is running when an `error` or `unhandledrejection` event reaches the global scope. A worker error the parent leaves unhandled is re-reported there, so specs that make a worker throw on purpose now cancel it on the parent (onerror returns true, or preventDefault() after the assertions). The Performance observer spec throws from an observer by design; it now runs under jasmine.spyOnGlobalErrorsAsync where that exists and keeps the preventDefault listener everywhere. done(reason) never failed a spec on Jasmine 2.0.1, so the worker and BroadcastChannel failure branches passed silently. They now go through failSpec(), which uses done.fail where available and a failing expectation otherwise. The worker stress spec finishes exactly once. All of it behaves the same on Jasmine 2.0.1.
1 parent 234f5f3 commit 2cb9c23

4 files changed

Lines changed: 110 additions & 36 deletions

File tree

‎BroadcastChannel/index.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ function closeOpenChannels() {
5353
});
5454
}
5555

56+
// done(reason) does not fail the spec on every Jasmine version.
57+
function failSpec(done, reason) {
58+
if (typeof done.fail === "function") {
59+
done.fail(reason);
60+
return;
61+
}
62+
expect("spec failed: " + reason).toBeUndefined();
63+
done();
64+
}
65+
5666
function expectThrowsTypeError(fn) {
5767
var thrown = null;
5868
try {
@@ -318,7 +328,11 @@ describe(module.id + " across isolates", function () {
318328

319329
function finish(failure) {
320330
worker.terminate();
321-
done(failure);
331+
if (failure === undefined) {
332+
done();
333+
} else {
334+
failSpec(done, failure);
335+
}
322336
}
323337

324338
channel.onmessage = function (event) {
@@ -328,6 +342,7 @@ describe(module.id + " across isolates", function () {
328342

329343
worker.onerror = function (e) {
330344
finish("worker error: " + (e && e.message));
345+
return true;
331346
};
332347

333348
worker.onmessage = function (msg) {

‎Performance/index.js‎

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,44 @@ function entryNames(entries) {
7575
}
7676

7777
// A callback that throws is routed to reportError, which surfaces as a global
78-
// `error` event. preventDefault() keeps it from reaching the runner's uncaught
79-
// handler for the one test that exercises that path.
80-
function suppressGlobalErrors() {
81-
if (typeof globalObject.addEventListener !== "function") {
82-
return function () {};
83-
}
78+
// `error` event. preventDefault() keeps it from reaching the runtime's uncaught
79+
// handler; where Jasmine routes global errors to the running spec, its global
80+
// error spy keeps the event from failing the spec too.
81+
// body(finish) must call finish only after the expected error was dispatched.
82+
function withGlobalErrorsSuppressed(done, body) {
83+
var canListen = typeof globalObject.addEventListener === "function";
8484
var listener = function (event) {
8585
if (event && typeof event.preventDefault === "function") {
8686
event.preventDefault();
8787
}
8888
};
89-
globalObject.addEventListener("error", listener);
90-
return function () {
91-
if (typeof globalObject.removeEventListener === "function") {
89+
if (canListen) {
90+
globalObject.addEventListener("error", listener);
91+
}
92+
function cleanup() {
93+
if (canListen && typeof globalObject.removeEventListener === "function") {
9294
globalObject.removeEventListener("error", listener);
9395
}
94-
};
96+
}
97+
98+
if (typeof jasmine.spyOnGlobalErrorsAsync !== "function") {
99+
body(function () {
100+
cleanup();
101+
done();
102+
});
103+
return;
104+
}
105+
jasmine.spyOnGlobalErrorsAsync(function () {
106+
return new Promise(function (resolve) {
107+
body(resolve);
108+
});
109+
}).then(function () {
110+
cleanup();
111+
done();
112+
}, function (error) {
113+
cleanup();
114+
done.fail(error);
115+
});
95116
}
96117

97118
describe("Performance globals", function () {
@@ -746,23 +767,23 @@ describe("PerformanceObserver", function () {
746767
});
747768

748769
it("Should run every observer in registration order even when one throws", function (done) {
749-
var order = [];
750-
var restoreErrorHandling = suppressGlobalErrors();
751-
752-
observing(function () { order.push("first"); }).observe({ entryTypes: ["mark"] });
753-
observing(function () {
754-
order.push("second");
755-
throw new Error("observer callback failure");
756-
}).observe({ entryTypes: ["mark"] });
757-
observing(function () { order.push("third"); }).observe({ entryTypes: ["mark"] });
758-
759-
performance.mark("fan-out");
760-
761-
setTimeout(function () {
762-
expect(order).toEqual(["first", "second", "third"]);
763-
restoreErrorHandling();
764-
done();
765-
}, 500);
770+
withGlobalErrorsSuppressed(done, function (finish) {
771+
var order = [];
772+
773+
observing(function () { order.push("first"); }).observe({ entryTypes: ["mark"] });
774+
observing(function () {
775+
order.push("second");
776+
throw new Error("observer callback failure");
777+
}).observe({ entryTypes: ["mark"] });
778+
observing(function () { order.push("third"); }).observe({ entryTypes: ["mark"] });
779+
780+
performance.mark("fan-out");
781+
782+
setTimeout(function () {
783+
expect(order).toEqual(["first", "second", "third"]);
784+
finish();
785+
}, 500);
786+
});
766787
});
767788
});
768789

‎WorkerEvents/index.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ describe(module.id, function () {
226226
expect(event.error).toBe(null);
227227
// A handler that returns nothing leaves the error unhandled.
228228
expect(event.defaultPrevented).toBe(false);
229+
// Left unhandled, the error is re-reported on the main global
230+
// scope, where Jasmine's global error handler fails the spec.
231+
event.preventDefault();
229232
worker.terminate();
230233
done();
231234
});
@@ -243,6 +246,7 @@ describe(module.id, function () {
243246
expect(typeof event.lineno).toBe("number");
244247
worker.terminate();
245248
done();
249+
return true;
246250
};
247251
worker.postMessage("go");
248252
});

‎Workers/index.js‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@ describe("TNS Workers", () => {
2626

2727
var gC = global.NSObject ? __collect : gc;
2828

29+
// A worker error that the parent's handlers leave unhandled is re-reported on
30+
// the main global scope, where Jasmine's global error handler fails whichever
31+
// spec is running. Specs that make a worker throw on purpose cancel it on the
32+
// parent.
33+
34+
// done(reason) does not fail the spec on every Jasmine version.
35+
function failSpec(done, reason) {
36+
if (typeof done.fail === "function") {
37+
done.fail(reason);
38+
return;
39+
}
40+
expect("spec failed: " + reason).toBeUndefined();
41+
done();
42+
}
43+
2944
it("Should have self property equal to global", (done) => {
3045
var worker = new Worker("./EvalWorker");
3146

@@ -48,6 +63,7 @@ describe("TNS Workers", () => {
4863
expect(e).not.toEqual(null);
4964
worker.terminate();
5065
done();
66+
return true;
5167
}
5268
});
5369
}
@@ -193,6 +209,7 @@ describe("TNS Workers", () => {
193209

194210
it("Should keep the worker alive after error", (done) => {
195211
var worker = new Worker("./EvalWorker.js");
212+
worker.onerror = () => true;
196213

197214
worker.postMessage({ eval: "throw new Error('just an error');" });
198215
worker.postMessage({ eval: "postMessage('pong');" });
@@ -438,6 +455,7 @@ describe("TNS Workers", () => {
438455
var onerrorCounter = 0;
439456
worker.onerror = (err) => {
440457
onerrorCounter++;
458+
return true;
441459
};
442460

443461
var onmessageCounter = 0;
@@ -515,6 +533,7 @@ describe("TNS Workers", () => {
515533
var onerrorCalled = false;
516534
worker.onerror = (err) => {
517535
onerrorCalled = true;
536+
return true;
518537
};
519538

520539
var lastReceivedMessage;
@@ -535,24 +554,35 @@ describe("TNS Workers", () => {
535554
var worker = new Worker("./WorkerStressJSTest.js");
536555
// Worker is not guaranteed to have finished before the check for runtimes leak, so track it manually
537556
expectedAliveRuntimes++;
557+
var finished = false;
558+
function finish(failure) {
559+
if (finished) {
560+
return;
561+
}
562+
finished = true;
563+
worker.terminate();
564+
if (failure === undefined) {
565+
done();
566+
} else {
567+
failSpec(done, failure);
568+
}
569+
}
538570
// the specific worker will post a message if something isn't right
539571
worker.onmessage = (msg) => {
540572
// Worker sends this message when it finishes successfully
541573
expectedAliveRuntimes--;
542574
if (msg.data !== "end") {
543-
worker.terminate();
544-
done("Exception is thrown in the web worker: " + msg);
575+
finish("Exception is thrown in the web worker: " + msg);
545576
}
546577
}
547578
worker.onerror = (e) => {
548579
expectedAliveRuntimes--;
549-
worker.terminate();
550-
done("Exception is thrown in the web worker: " + e);
580+
finish("Exception is thrown in the web worker: " + e.message);
581+
return true;
551582
}
552583

553584
setTimeout(() => {
554-
worker.terminate();
555-
done();
585+
finish();
556586
}, DEFAULT_TIMEOUT_BEFORE_ASSERT);
557587
});
558588

@@ -627,6 +657,7 @@ describe("TNS Workers", () => {
627657
worker.onerror = (err) => {
628658
worker.terminate();
629659
done();
660+
return true;
630661
};
631662
});
632663

@@ -643,6 +674,7 @@ describe("TNS Workers", () => {
643674
worker.onerror = (err) => {
644675
worker.terminate();
645676
done();
677+
return true;
646678
}
647679
});
648680

@@ -766,7 +798,8 @@ describe("TNS Workers", () => {
766798

767799
worker.onerror = (e) => {
768800
console.log("NESTED WORKER TEST ERROR: " + e.message + " | " + e.stackTrace);
769-
done("nested worker error: " + e.message);
801+
failSpec(done, "nested worker error: " + e.message);
802+
return true;
770803
};
771804
worker.onmessage = (msg) => {
772805
expect(msg.data).toBe("child says: hello from nested worker");
@@ -787,7 +820,8 @@ describe("TNS Workers", () => {
787820

788821
worker.onerror = (e) => {
789822
console.log("NESTED WORKER TEST ERROR: " + e.message + " | " + e.stackTrace);
790-
done("nested worker error: " + e.message);
823+
failSpec(done, "nested worker error: " + e.message);
824+
return true;
791825
};
792826
worker.onmessage = (msg) => {
793827
var childThreadName = msg.data;

0 commit comments

Comments
 (0)