Skip to content

Commit

Permalink
Tests: Fix flaky dump test "Circular reference with arrays"
Browse files Browse the repository at this point in the history
There were two issues here:

* The test relied on non-default maxDepth of null or > 10 since
  its example involves recurion 10 levels deep, and the default
  maxDepth is 5.

* There is a bug in `QUnit.dump.parse()` where, if the input value
  cannot be mapped to a formatter function, it returns a function
  instead of a string.

  This bug was introduced nearly ten years ago in commit 818b2be,
  where `error` was replaced with a function that had a different
  purpose than what was there before, and this one use of it as
  fallback was not updated.

  Unfortunately, I am unable to say what value causes this to happen.
  I can consistently reproduce the issue in Safari 9.1 via BrowserStack,
  but only if I don't open the console, don't run a subset of the test,
  and don't modify any of the surrounding code. I have tried dozens
  upon dozens of clever ways to store values during the parse() function
  call in temporary variables and alert() them but as soon as I tried
  to capture anything that would help, the bug would stop happening.

  I can only conclude that there must be some kind of memory or JIT
  corruption in the JavaScript engine of Safari 9.1 that has some
  extremely specific conditions attached to it. Conditions that we
  did not meet before the last few commits, and are meeting now.

```
[OS X El Capitan, Safari 9.1] Error: "Circular reference with arrays" failed
Died on test #1 @http://localhost:8899/test/main/dump.js:279:11:
expected.indexOf is not a function.
(In 'expected.indexOf("[object Array]")', 'expected.indexOf' is undefined)
```
  • Loading branch information
Krinkle committed Jun 7, 2021
1 parent b40a0f1 commit b925241
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/dump.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ export default ( function() {
stack.pop();
return res;
}
return ( parserType === "string" ) ? parser : this.parsers.error;
if ( parserType === "string" ) {
return parser;
}
return "[ERROR: Missing QUnit.dump formatter for type " + objType + "]";
},
typeOf: function( obj ) {
let type;
Expand Down Expand Up @@ -180,6 +183,9 @@ export default ( function() {
error: function( error ) {
return "Error(\"" + error.message + "\")";
},

// This has been unused since QUnit 1.0.0.
// @todo Deprecate and remove.
unknown: "[Unknown]",
"null": "null",
"undefined": "undefined",
Expand Down
6 changes: 6 additions & 0 deletions test/main/dump.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

QUnit.module( "dump", {
afterEach: function() {

// Restore default
QUnit.dump.maxDepth = null;
}
} );
Expand Down Expand Up @@ -239,6 +241,8 @@ function chainwrap( depth, first, prev ) {
QUnit.test( "Check dump recursion", function( assert ) {
var noref, nodump, selfref, selfdump, parentref, parentdump, circref, circdump;

// QUnit.dump.maxDepth = 20;

noref = chainwrap( 0 );
nodump = QUnit.dump.parse( noref );
assert.equal( nodump, "{\n \"first\": true,\n \"wrap\": undefined\n}" );
Expand All @@ -263,6 +267,8 @@ QUnit.test( "Check dump recursion", function( assert ) {
QUnit.test( "Check equal/deepEqual recursion", function( assert ) {
var noRecursion, selfref, circref;

// QUnit.dump.maxDepth = 20;

noRecursion = chainwrap( 0 );
assert.equal( noRecursion, noRecursion, "I should be equal to me." );
assert.deepEqual( noRecursion, noRecursion, "... and so in depth." );
Expand Down

0 comments on commit b925241

Please sign in to comment.