Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show diff only when it helps #802

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 30 additions & 12 deletions reporter/html.js
Expand Up @@ -635,9 +635,15 @@ QUnit.testStart(function( details ) {

});

function stripHtml( string ) {
// strip tags, html entity and whitespaces
return string.replace(/<\/?[^>]+(>|$)/g, "").replace(/\&quot;/g, "").replace(/\s+/g, "");
}

QUnit.log(function( details ) {
var assertList, assertLi,
message, expected, actual,
message, expected, actual, diff,
showDiff = false,
testItem = id( "qunit-test-output-" + details.testId );

if ( !testItem ) {
Expand All @@ -659,19 +665,31 @@ QUnit.log(function( details ) {
"</pre></td></tr>";

if ( actual !== expected ) {

message += "<tr class='test-actual'><th>Result: </th><td><pre>" +
actual + "</pre></td></tr>" +
"<tr class='test-diff'><th>Diff: </th><td><pre>" +
QUnit.diff( expected, actual ) + "</pre></td></tr>";
} else {
if ( expected.indexOf( "[object Array]" ) !== -1 ||
expected.indexOf( "[object Object]" ) !== -1 ) {
message += "<tr class='test-message'><th>Message: </th><td>" +
"Diff suppressed as the depth of object is more than current max depth (" +
QUnit.config.maxDepth + ").<p>Hint: Use <code>QUnit.dump.maxDepth</code> to " +
" run with a higher max depth or <a href='" + setUrl({ maxDepth: -1 }) + "'>" +
"Rerun</a> without max depth.</p></td></tr>";
actual + "</pre></td></tr>";

// Don't show diff if actual or expected are booleans
if ( !( /^(true|false)$/.test( actual ) ) &&
!( /^(true|false)$/.test( expected ) ) ) {
diff = QUnit.diff( expected, actual );
showDiff = stripHtml( diff ).length !==
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be false when the diff is exactly as long as actual length plus expected length, right? Is that really a good measure?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QUnit.diff( "1234567890", "abcdefghij" );
"<del>1234567890</del><ins>abcdefghij</ins>"

AFAIK, this happens when there's no diff, the objects are completely different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it makes sense when we already expose the actual and expected and there's nothing equal from them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, we can tune it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @leobalter , if objects are completely different then only the condition will be false.

stripHtml( expected ).length +
stripHtml( actual ).length;
}

// Don't show diff if expected and actual are totally different
if ( showDiff ) {
message += "<tr class='test-diff'><th>Diff: </th><td><pre>" +
diff + "</pre></td></tr>";
}
} else if ( expected.indexOf( "[object Array]" ) !== -1 ||
expected.indexOf( "[object Object]" ) !== -1 ) {
message += "<tr class='test-message'><th>Message: </th><td>" +
"Diff suppressed as the depth of object is more than current max depth (" +
QUnit.config.maxDepth + ").<p>Hint: Use <code>QUnit.dump.maxDepth</code> to " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix the <code>QUnit.dump.maxDepth</code> to use QUnit.config.maxDepth as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried to change it to QUnit.config.maxDepth but strangely it din't work, whereas using QUnit.dump.maxDepth works fine. I guess this is happening because dump.maxDepth is initialized with default config.maxDepth in file dump.js and then the value of config.maxDepth is changed. Hence I got a message which looks like:

Diff suppressed as the depth of object is more than current max depth (infinity)

However specifying maxDepth in GET parameters works fine because if parameters are present, the value of config.maxDepth is updated in file core.js before initialization in dump.js.

" run with a higher max depth or <a href='" + setUrl({ maxDepth: -1 }) + "'>" +
"Rerun</a> without max depth.</p></td></tr>";
}

if ( details.source ) {
Expand Down