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

feat: support reporting AggregateErrors #5018

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file modified bin/mocha.js
100644 → 100755
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Still the permissions change? Is there an automatic modification happening on save or something like that?

Copy link
Author

Choose a reason for hiding this comment

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

It's possible. I'll look into it this week.

Empty file.
1 change: 1 addition & 0 deletions lib/cli/run-option-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ exports.aliases = {
ignore: ['exclude'],
invert: ['i'],
jobs: ['j'],
jsonStringifyWhitespace: ['jsw'],
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
'no-colors': ['C'],
'node-option': ['n'],
parallel: ['p'],
Expand Down
31 changes: 20 additions & 11 deletions lib/reporters/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,11 @@ exports.list = function (failures) {
multipleErr = [test.err].concat(test.err.multiple);
}
err = multipleErr.shift();
} else {
} else if (test.err) {
err = test.err;
} else {
// Handles when failures is a list of errors and not test objects.
err = test;
Comment on lines +301 to +305
Copy link
Member

Choose a reason for hiding this comment

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

[Style] ✨

Suggested change
} else if (test.err) {
err = test.err;
} else {
// Handles when failures is a list of errors and not test objects.
err = test;
} else if (test.err) {
// Handles when failures is a list of errors and not test objects.
err = test.err || test;

}
var message;
if (typeof err.inspect === 'function') {
Expand Down Expand Up @@ -292,17 +295,24 @@ exports.list = function (failures) {

// indented test title
var testTitle = '';
test.titlePath().forEach(function (str, index) {
if (index !== 0) {
testTitle += '\n ';
}
for (var i = 0; i < index; i++) {
testTitle += ' ';
}
testTitle += str;
});
// Incase test is an AggregateError object when recursively listing errors
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
if (test instanceof AggregateError) {
test.titlePath().forEach(function (str, index) {
if (index !== 0) {
testTitle += '\n ';
}
for (var i = 0; i < index; i++) {
testTitle += ' ';
}
testTitle += str;
});
}

CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
Base.consoleLog(fmt, i + 1, testTitle, msg, stack);
// Handle Aggregate Errors
if (test.err && test.err.errors) {
Base.list(test.err.errors);
}
});
};

Expand Down Expand Up @@ -544,7 +554,6 @@ var objToString = Object.prototype.toString;
function sameType(a, b) {
return objToString.call(a) === objToString.call(b);
}

CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
Base.consoleLog = consoleLog;

Base.abstract = true;
39 changes: 26 additions & 13 deletions test/reporters/base.spec.js
Copy link
Member

Choose a reason for hiding this comment

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

@CheadleCheadle there are tests failing, could you please take a look?

Note that I just pulled the latest changes in from main (renamed from master). They didn't fix the test failure.

Original file line number Diff line number Diff line change
Expand Up @@ -507,21 +507,34 @@ describe('Base reporter', function () {
);
});

describe('when reporter output immune to user test changes', function () {
var baseConsoleLog;
it('should list all the errors within an AggregateError', function () {
var err1 = new Error('1');
var err2 = new Error('2');
var aggErr = new AggregateError([err1, err2], '2 errors');
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved

beforeEach(function () {
sinon.restore();
sinon.stub(console, 'log');
baseConsoleLog = sinon.stub(Base, 'consoleLog');
});
var test = makeTest(aggErr);
list([test]);

it('should let you stub out console.log without effecting reporters output', function () {
Base.list([]);
baseConsoleLog.restore();
var errOut = stdout.join('\n').trim();

expect(baseConsoleLog, 'was called');
expect(console.log, 'was not called');
});
expect(errOut, 'to contain', ' Error: 1', 'Error: 2');
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('when reporter output immune to user test changes', function () {
var baseConsoleLog;

beforeEach(function () {
sinon.restore();
sinon.stub(console, 'log');
baseConsoleLog = sinon.stub(Base, 'consoleLog');
});

it('should let you stub out console.log without effecting reporters output', function () {
Base.list([]);
baseConsoleLog.restore();

expect(baseConsoleLog, 'was called');
expect(console.log, 'was not called');
});
});
Loading