v2.16.0+ reports wrong source in browser

[phanirithvij]

Tell us about your runtime:

• QUnit version: 2.16.0 (and after) can also reproduce the bug with 2.18.0
• Which environment are you using? (e.g., browser, Node): browser
• How are you running QUnit? (e.g., QUnit CLI, Grunt, Karma, manually in browser): manually in browser

What are you trying to do?

Code that reproduces the problem: https://github.com/phanirithvij/tarballjs check the tests/ folder

I've added tests for both 2.15.0 and 2.16.0 in the tests/ folder in the repo I've provided.

They are accessible via github pages here.

• https://phanirithvij.github.io/tarballjs/tests/2.16.0/
• https://phanirithvij.github.io/tarballjs/tests/2.15.0/

Regression in 2.15.0...2.16.0

What did you expect to happen?

Sources should be reported correctly (works in 2.15.0)

// In https://phanirithvij.github.io/tarballjs/tests/2.15.0/
Source: at https://phanirithvij.github.io/tarballjs/tests/2.15.0/tests.js:44:7

Screenshot

What actually happened?

From version 2.16.0 onwards the source file is being reported as

// https://phanirithvij.github.io/tarballjs/tests/2.16.0/
Source: at Object.test (https://code.jquery.com/qunit/qunit-2.16.0.js:2751:5)

Screenshot

[Krinkle]

Thank you for the clear examples, that helps a lot. I've confirmed the bug.

To clarify, this is about the source attribution for passing tests, indicating where the test block starts. It is not about the stack trace for failing tests, which seems to be unaffected.

I'll also note that while this bug appears consistent on your demo, I do note that in other cirumstances the source is still correctly reported. For example, running QUnit's own tests, it reports:

1. equiv: Primitive types and constants (95)
   Source: test@http://localhost:9412/qunit/qunit.js:3614:12
   @http://localhost:9412/test/main/deepEqual.js:121:7

Which shows both URLs. The former of which isn't useful and that's a regression there as well, but might explain why it wasn't noticed earlier. Sorry about that. I'll get this fixed right away.

[Krinkle]

That was a fun little investigation. This is mostly for myself, but I'll post it here regardless.

Original stack trace

Upon defining a test case via QUnit.test(), we record the current stack trace. For most test suites, this looks as follows:

Test@/lib/qunit/qunit-2.15.0.js # internals of "new Test()"
test@/lib/qunit/qunit-2.15.0.js # internals of "test()" call "new Test()"
@/test/my-app.js:44:7 # You call test()

Test@Test@/lib/qunit/qunit-2.16.0.js
addTest@/lib/qunit/qunit-2.16.0.js
test@/lib/qunit/qunit-2.16.0.js
@/test/my-app.js:44:7

Note that the stack trace simply starts at the bottom with your test suite, and then enters QUnit internals. Those higher frames are not of interest to the developer.

If you use nested modules, then the stack might go a bit further:

Test@Test@/lib/qunit/qunit.js
test@/lib/qunit/qunit.js
@/test/my-app.js:44:7 # You call test()
module@/lib/qunit/qunit.js
@/test/my-app.js:1:7 # You call module()

Users also sometimes use utility function for defining multiple tests (although we now have QUnit.test.each):

function myUtility(label, data) {
  QUnit.test(label, function (assert) {
     ...data...
  });
}
myUtility('one', ..);
myUtility('two', ..);

Test@Test@/lib/qunit/qunit.js
test@/lib/qunit/qunit.js
@/test/my-app.js:12:12 # You call test(label)
@/test/my-app.js:44:7 # You call myUtility('one')
module@/lib/qunit/qunit.js
@/test/my-app.js:1:7 # You call module()

Lastly, when testing asynchronous code, modern browsers will provide the details of where the first execution started, which can be the browser onLoad event, or the previous unrelated test fulfilling its Promise.

The point is: Only a specific specific range in the trace is relevant. We want to skip the top frames that are internal. Then keep the ones from your code. And then then skip everything else, even if it goes back to your code, since that will be previous unrelated code now.

The way it worked

When the user interface asks for Source line, we compute it by analyzing the stacktrace. The default HTML Reporter does this. The QUnit CLI, TAP Reporter, and most cloud/CI integrations do not do this, as they tend to only report failed tests or provide only the name of the test for progress reporting.

The analysis is quite simple (source: src/core/stracktrace.js):

1. Skip the first 2 frames because we know they are internal. This is the first sign of trouble, because in QUnit 2.16 we have one extra frame.
2. Display the next frames, until we see another frame that looks internal. Then we stop and don't look after that.

The problem is of course that the code immediately thought that there were 0 frames of your code, and then the third frame is the "start" of more internal frames.