Bug 1856464 - Include stack in Assert.rejects r=jmaher

Assert.rejects uses callbacks in a way that the stack no longer includes
the original caller in the stack when the stack is generated.
Consequently, when AssertionError tries to identify the caller, it is
unable to do so, resulting in a `null` stack.

This `null` stack is passed by the SpecialPowers glue to
`do_report_result` in xpcshell/head.js, which doesn't expect a `null`
stack, and consequently the following error is reported:
"TypeError: can't access property "filename", stack is null"

To fix this issue, we save the stack (including the caller) upon
entering `Assert.rejects`, and forward that to `AssertionError`.

Differential Revision: https://phabricator.services.mozilla.com/D189863
This commit is contained in:
Rob Wu 2023-10-25 12:48:07 +00:00
Родитель ea35974d0b
Коммит 30534ae574
3 изменённых файлов: 40 добавлений и 5 удалений

Просмотреть файл

@ -129,6 +129,18 @@ add_task(async function() {
let results = diags.map(diag => [diag.condition, diag.name, diag.diag]);
isDeeply(results, expected, "Got expected assertions");
for (let { name: diagName, stack } of diags) {
ok(stack, `Got stack for: ${diagName}`);
let expectedFilenamePart = "/test_SpecialPowersSandbox.html:";
if (name === "SpecialPowers.loadChromeScript") {
// Unfortunately, the original file name is not included;
// the function name or a dummy value is used instead.
expectedFilenamePart = "loadChromeScript anonymous function>";
}
if (!stack.includes(expectedFilenamePart)) {
ok(false, `Stack does not contain ${expectedFilenamePart}: ${stack}`);
}
}
}
});
</script>

Просмотреть файл

@ -146,5 +146,17 @@ add_task(async function () {
let results = diags.map(diag => [diag.passed, diag.msg]);
deepEqual(results, expected, "Got expected assertions");
for (let { msg, stack } of diags) {
ok(stack, `Got stack for: ${msg}`);
let expectedFilenamePart = "/test_SpecialPowersSandbox.js:";
if (name === "SpecialPowers.loadChromeScript") {
// Unfortunately, the original file name is not included;
// the function name or a dummy value is used instead.
expectedFilenamePart = "loadChromeScript anonymous function>:";
}
if (!stack.includes(expectedFilenamePart)) {
ok(false, `Stack does not contain ${expectedFilenamePart}: ${stack}`);
}
}
}
});

Просмотреть файл

@ -112,7 +112,8 @@ function getMessage(error, prefix = "") {
* actual: actual,
* expected: expected,
* operator: operator,
* truncate: truncate
* truncate: truncate,
* stack: stack, // Optional, defaults to the current stack.
* });
*
*/
@ -123,7 +124,7 @@ Assert.AssertionError = function (options) {
this.operator = options.operator;
this.message = getMessage(this, options.message, options.truncate);
// The part of the stack that comes from this module is not interesting.
let stack = Components.stack;
let stack = options.stack || Components.stack;
do {
stack = stack.asyncCaller || stack.caller;
} while (
@ -205,6 +206,9 @@ Assert.prototype.setReporter = function (reporterFunc) {
* Operation qualifier used by the assertion method (ex: '==').
* @param {boolean} [truncate=true]
* Whether or not ``actual`` and ``expected`` should be truncated when printing.
* @param {nsIStackFrame} [stack]
* The stack trace including the caller of the assertion method,
* if this cannot be inferred automatically (e.g. due to async callbacks).
*/
Assert.prototype.report = function (
failed,
@ -212,7 +216,8 @@ Assert.prototype.report = function (
expected,
message,
operator,
truncate = true
truncate = true,
stack = null // Defaults to Components.stack in AssertionError.
) {
// Although not ideal, we allow a "null" message due to the way some of the extension tests
// work.
@ -228,6 +233,7 @@ Assert.prototype.report = function (
expected,
operator,
truncate,
stack,
});
if (!this._reporter) {
// If no custom reporter is set, throw the error.
@ -513,6 +519,8 @@ Assert.prototype.throws = function (block, expected, message) {
*/
Assert.prototype.rejects = function (promise, expected, message) {
checkExpectedArgument(this, "rejects", expected);
const operator = undefined; // Should we use "rejects" here?
const stack = Components.stack;
return new Promise((resolve, reject) => {
return promise
.then(
@ -521,7 +529,10 @@ Assert.prototype.rejects = function (promise, expected, message) {
true,
null,
expected,
"Missing expected exception " + message
"Missing expected exception " + message,
operator,
true,
stack
);
// this.report() above should raise an AssertionError. If _reporter
// has been overridden and doesn't throw an error, just resolve.
@ -534,7 +545,7 @@ Assert.prototype.rejects = function (promise, expected, message) {
reject(err);
return;
}
this.report(false, err, expected, message);
this.report(false, err, expected, message, operator, truncate, stack);
resolve();
}
)