Reland "[mjsunit] Improve mjsunit stracktrace readability"

This is a reland of f720d024dc
Original change's description:
> [mjsunit] Improve mjsunit stracktrace readability
> 
> Format the function name and file-position into proper columns to easily spot
> where the test code ends and the mjsunit framework code starts.
> 
> BEFORE:
> Stack: Error
>     at new MjsUnitAssertionError (test/mjsunit/mjsunit.js:36:18)
>     at failWithMessage (test/mjsunit/mjsunit.js:310:11)
>     at fail (test/mjsunit/mjsunit.js:327:12)
>     at assertEquals (test/mjsunit/mjsunit.js:398:7)
>     at closure (test/mjsunit/regress/regress-4121.js:20:7)
>     at literals_sharing_test (test/mjsunit/regress/regress-4121.js:27:3)
>     at test (test/mjsunit/regress/regress-4121.js:37:5)
>     at eval (eval at <anonymous> (test/mjsunit/regress/regress-4121.js:49:6), <anonymous>:1:1)
>     at test/mjsunit/regress/regress-4121.js:49:6
>     at Array.forEach.call (test/mjsunit/regress/regress-4121.js:50:7)
>     throw new MjsUnitAssertionError(message);
> 
> AFTER:
> Stack: MjsUnitAssertionError
>     at assertEquals          test/mjsunit/mjsunit.js 398:7
>     at closure               test/mjsunit/regress/regress-4121.js 20:7
>     at literals_sharing_test test/mjsunit/regress/regress-4121.js 27:3
>     at test                  test/mjsunit/regress/regress-4121.js 37:5
>     at eval                  eval at <anonymous> (test/mjsunit/regress/regress-4121.js:49:6)
>     at                       test/mjsunit/regress/regress-4121.js 49:6
>     at Array.forEach.call    test/mjsunit/regress/regress-4121.js 50:7
>     throw new MjsUnitAssertionError(message);
> 
> 
> Change-Id: Iad3460a648e26effb43c00426ab043743ee6a138
> Reviewed-on: https://chromium-review.googlesource.com/563627
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46589}

Change-Id: I44bf07f7be4114369315605542cafd17345b4397
Reviewed-on: https://chromium-review.googlesource.com/567063
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46602}
This commit is contained in:
Camillo Bruni 2017-07-12 17:28:49 +02:00 committed by Commit Bot
parent 873d51673a
commit 34874b3b19
5 changed files with 182 additions and 3 deletions

View File

@ -0,0 +1,102 @@
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
let fileName = 'mjsunit-assertion-error.js';
function addDefaultFrames(frameExpectations) {
// The last frame contains the error instantiation.
frameExpectations.unshift('assertTrue.*mjsunit.js');
// Frist frame is the top-level script.
frameExpectations.push(fileName);
}
function assertErrorMessage(frameExpectations, error) {
let stack = error.stack.split("\n");
let title = stack.shift();
assertContains('MjsUnitAssertionError', title);
addDefaultFrames(frameExpectations);
// Add default frames to the expectations.
assertEquals(frameExpectations.length, stack.length);
for (let i = 0; i < stack.length; i++) {
let frame = stack[i];
let expectation = frameExpectations[i];
assertTrue(frame.search(expectation) != -1,
`Frame ${i}: Did not find '${expectation}' in '${frame}'`);
}
}
// Toplevel
try {
assertTrue(false);
} catch(e) {
assertErrorMessage([], e);
}
// Single function.
function throwError() {
assertTrue(false);
}
try {
throwError();
assertUnreachable();
} catch(e) {
assertErrorMessage(['throwError'], e);
}
// Nested function.
function outer() {
throwError();
}
try {
outer();
assertUnreachable();
} catch(e) {
assertErrorMessage(['throwError', 'outer'], e);
}
// Test Array helper nesting
try {
[1].map(throwError);
assertUnreachable();
} catch(e) {
assertErrorMessage(['throwError', 'Array.map'], e);
}
try {
Array.prototype.map.call([1], throwError);
assertUnreachable();
} catch(e) {
assertErrorMessage(['throwError', 'Array.map'], e);
}
// Test eval
try {
eval("assertTrue(false);");
assertUnreachable();
} catch(e) {
assertErrorMessage(['eval'], e);
}
(function testNestedEval() {
try {
eval("assertTrue(false);");
assertUnreachable();
} catch(e) {
assertErrorMessage(['eval', 'testNestedEval'], e);
}
})();
(function testConstructor() {
class Failer {
constructor() {
assertTrue(false);
}
}
try {
new Failer();
assertUnreachable();
} catch(e) {
assertErrorMessage(['new Failer', 'testConstructor'], e);
}
})();

View File

@ -27,17 +27,85 @@
function MjsUnitAssertionError(message) {
this.message = message;
// This allows fetching the stack trace using TryCatch::StackTrace.
this.stack = new Error("").stack;
// Temporarily install a custom stack trace formatter and restore the
// previous value.
let prevPrepareStackTrace = Error.prepareStackTrace;
try {
Error.prepareStackTrace = MjsUnitAssertionError.prepareStackTrace;
// This allows fetching the stack trace using TryCatch::StackTrace.
this.stack = new Error("MjsUnitAssertionError").stack;
} finally {
Error.prepareStackTrace = prevPrepareStackTrace;
}
}
// Custom V8-specific stack trace formatter that is temporarily installed on
// the Error object.
MjsUnitAssertionError.prepareStackTrace = function(error, stack) {
// Trigger default formatting with recursion.
try {
// Filter-out all but the first mjsunit frame.
let filteredStack = [];
let inMjsunit = true;
for (let i = 0; i < stack.length; i++) {
let frame = stack[i];
if (inMjsunit) {
let file = frame.getFileName();
if (!file || !file.endsWith("mjsunit.js")) {
inMjsunit = false;
// Push the last mjsunit frame, typically containing the assertion
// function.
if (i > 0) filteredStack.push(stack[i-1]);
filteredStack.push(stack[i]);
}
continue;
}
filteredStack.push(frame);
}
stack = filteredStack;
// Infer function names and calculate {max_name_length}
let max_name_length = 0;
stack.forEach(each => {
let name = each.getFunctionName();
if (name == null) name = "";
if (each.isEval()) {
name = name;
} else if (each.isConstructor()) {
name = "new " + name;
} else if (each.isNative()) {
name = "native " + name;
} else if (!each.isToplevel()) {
name = each.getTypeName() + "." + name;
}
each.name = name;
max_name_length = Math.max(name.length, max_name_length)
});
// Format stack frames.
stack = stack.map(each => {
let frame = " at " + each.name.padEnd(max_name_length);
let fileName = each.getFileName();
if (each.isEval()) return frame + " " + each.getEvalOrigin();
frame += " " + (fileName ? fileName : "");
let line= each.getLineNumber();
frame += " " + (line ? line : "");
let column = each.getColumnNumber();
frame += (column ? ":" + column : "");
return frame;
});
return "" + error.message + "\n" + stack.join("\n");
} catch(e) {};
return error.stack;
}
/*
* This file is included in all mini jsunit test cases. The test
* framework expects lines that signal failed tests to start with
* the f-word and ignore all other lines.
*/
MjsUnitAssertionError.prototype.toString = function () {
return this.message + "\n\nStack: " + this.stack;
};

View File

@ -649,4 +649,11 @@
'whitespaces': [SKIP],
}], # variant == wasm_traps
##############################################################################
['no_harness', {
# skip assertion tests since the stack trace is broken if mjsunit is
# included in the snapshot
'mjsunit-assertion-error' : [SKIP],
}], # no_harness
]

View File

@ -405,6 +405,7 @@ def Execute(arch, mode, args, options, suites, workspace):
"novfp3": False,
"predictable": False,
"byteorder": sys.byteorder,
"no_harness": False,
}
all_tests = []
num_tests = 0

View File

@ -836,6 +836,7 @@ def Execute(arch, mode, args, options, suites):
"novfp3": options.novfp3,
"predictable": options.predictable,
"byteorder": sys.byteorder,
"no_harness": options.no_harness
}
all_tests = []
num_tests = 0