From 52f37b8fcb58fc258c1644bf17dbf7a5fdcc471a Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 5 Jul 2024 05:40:42 +0100 Subject: [PATCH] Core: Fix missing second frame in QUnit.stack() in Safari Safari implements ES6 Tail-Call Optimization, which is when: - the file is in strict mode, - and, in a regular function (not async or generator), - and, the return statement ends in a simple function call. Then, the current function is removed from the stack before the child function begins. TCO applies even for calls that are not recursive. The result is that, given: > makeFakeFailure -> exampleMain -> exampleParent -> exampleCurrent -> > QUnit.stack -> sourceFromStacktrace -> new Error. In Firefox and Chrome, `e.stack` is: ``` [0] sourceFromStacktrace (SLICED) [1] QUnit.stack (SLICED) [2] exampleCurrent [3] exampleParent [4] exampleMain [5] makeFakeFailure ``` But, in Safari, the second frame gets lost because our tiny `QUnit.stack()` function is a candidate for Tail-Call Optimization. ``` [0] sourceFromStacktrace (SLICED) [1] exampleCurrent (SLICED) [2] exampleParent [3] exampleMain [4] makeFakeFailure ``` This, combined with the fact that we strip the first two frames as a way to hide internal offsets, meant that in Safari we ended up attributing failed assertions and test definitions to the parent of the caller rather than the actual caller, e.g. exampleParent() instead of exampleCurrent. Ref https://bugs.webkit.org/show_bug.cgi?id=276187. Closes https://github.com/qunitjs/qunit/pull/1776. --- src/core.js | 5 +++- test/main/stacktrace.js | 59 +++++++++++++++++------------------------ 2 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/core.js b/src/core.js index 4e9bd0e69..34d4ef0b9 100644 --- a/src/core.js +++ b/src/core.js @@ -96,7 +96,10 @@ extend(QUnit, { stack: function (offset) { offset = (offset || 0) + 2; - return sourceFromStacktrace(offset); + // Support Safari: Use temp variable to avoid TCO for consistent cross-browser result + // https://bugs.webkit.org/show_bug.cgi?id=276187 + const source = sourceFromStacktrace(offset); + return source; } }); diff --git a/test/main/stacktrace.js b/test/main/stacktrace.js index 5b1ebebe6..6767f29b7 100644 --- a/test/main/stacktrace.js +++ b/test/main/stacktrace.js @@ -6,22 +6,33 @@ function fooParent () { return fooCurrent(); } + function fooMain () { + return fooParent(); + } - function fooInternal () { + function barInternal () { return QUnit.stack(2); } - function fooPublic () { - return fooInternal(); + function barPublic () { + return barInternal(); } - function barCaller () { - return fooPublic(); + function quuxCaller () { + return barPublic(); } function norm (str) { - // CRLF + // Windows path return str.replace(/\\/g, '/'); } + function shorten (str) { + return norm(str) + // Remove browser-specific formatting and line numbers + .replace(/^.*((?:foo|bar|quux)[A-z]+).*$/gm, '$1') + // Remove anything below our entry point + .replace(/(fooMain|quuxCaller)[\s\S\n]*/, '$1'); + } + QUnit.test('QUnit.stack()', function (assert) { var simple = norm(QUnit.stack()); assert.pushResult({ @@ -36,38 +47,18 @@ message: 'stacktrace cleaning stops before qunit.js' }); - var nested = norm(fooParent()); - assert.pushResult({ - result: nested.indexOf('fooCurrent') !== -1, - actual: nested, - message: 'include current function' - }); - assert.pushResult({ - result: nested.indexOf('fooParent') !== -1, - actual: nested, - message: 'include parent function' - }); + var fooStack = shorten(fooMain()).split('\n'); + assert.deepEqual(fooStack, [ + 'fooCurrent', + 'fooParent', + 'fooMain' + ]); }); QUnit.test('QUnit.stack(offset)', function (assert) { - var stack = norm(barCaller()); - var line = stack.split('\n')[0]; + var barStack = shorten(quuxCaller()).split('\n'); - assert.pushResult({ - result: line.indexOf('/main/stacktrace.js') !== -1, - actual: line, - message: 'current file' - }); - assert.pushResult({ - result: line.indexOf('barCaller') !== -1, - actual: line, - message: 'start at offset' - }); - assert.pushResult({ - result: stack.indexOf('fooInternal') === -1, - actual: stack, - message: 'skip internals' - }); + assert.deepEqual(barStack, ['quuxCaller']); }); // Some browsers support 'stack' only when caught (see sourceFromStacktrace).