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.

Sign up for GitHub

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

Jump to bottom

spec-reporter prints retries #5099

Closed
wants to merge 11 commits into from
Closed

spec-reporter prints retries #5099

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5e25d51
spec-reporter prints retries
CheadleCheadle Feb 22, 2024
f7e337c
Merge branch 'master' into spec-reporter-retries
CheadleCheadle Feb 22, 2024
00b710c
Merge branch 'master' into spec-reporter-retries
CheadleCheadle Feb 26, 2024
b5d4d68
Merge branch 'master' into spec-reporter-retries
CheadleCheadle Mar 1, 2024
26881ee
refactor retries
CheadleCheadle Mar 7, 2024
9a4845b
Merge branch 'master' into spec-reporter-retries
CheadleCheadle Mar 7, 2024
2fdc32e
Merge branch 'master' into spec-reporter-retries
CheadleCheadle Mar 11, 2024
4423d2c
Merge branch 'master' into spec-reporter-retries
CheadleCheadle Mar 18, 2024
05bd4bc
Merge branch 'mochajs:main' into spec-reporter-retries
CheadleCheadle Jul 5, 2024
7431273
Merge branch 'main' into spec-reporter-retries
CheadleCheadle Jul 10, 2024
706c723
whitespace fixes
CheadleCheadle Jul 10, 2024
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
View file
Open in desktop
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
Empty file.
30 changes: 22 additions & 8 deletions lib/reporters/spec.js
View file
Open in desktop
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -45,46 +45,60 @@ function Spec(runner, options) {
return Array(indents).join(' ');
}

runner.on(EVENT_RUN_BEGIN, function () {
runner.on(EVENT_RUN_BEGIN, function() {
Base.consoleLog();
});

runner.on(EVENT_SUITE_BEGIN, function (suite) {
runner.on(EVENT_SUITE_BEGIN, function(suite) {
++indents;
Base.consoleLog(color('suite', '%s%s'), indent(), suite.title);
});

runner.on(EVENT_SUITE_END, function () {
runner.on(EVENT_SUITE_END, function() {
--indents;
if (indents === 1) {
Base.consoleLog();
}
});

runner.on(EVENT_TEST_PENDING, function (test) {
runner.on(EVENT_TEST_PENDING, function(test) {
var fmt = indent() + color('pending', ' - %s');
Base.consoleLog(fmt, test.title);
});

runner.on(EVENT_TEST_PASS, function (test) {
runner.on(EVENT_TEST_PASS, function(test) {
function logRetries() {
if (test._currentRetry > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

The underscore in _currentRetry is an old-school practice for indicating that its a private property, so we shouldn't rely on it here I think as its not intended for outside consumption.

If we want it to be exposed we should make it an explicit public property, like duration, and probably name it retries or retryCount or such

We should use the public .currentRetry() instead.

/*
Included message within format so it can be passed into `Base.consoleLog` as one argument
*/
var retryFmt = color(
'bright yellow',
Copy link
Member

Choose a reason for hiding this comment

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

To really stick with the theme I think we should add a 'retries' color here:

mocha/lib/reporters/base.js

Lines 64 to 90 in 24560c1

/**
* Default color map.
*/
exports.colors = {
pass: 90,
fail: 31,
'bright pass': 92,
'bright fail': 91,
'bright yellow': 93,
pending: 36,
suite: 0,
'error title': 0,
'error message': 31,
'error stack': 90,
checkmark: 32,
fast: 90,
medium: 33,
slow: 31,
green: 32,
light: 90,
'diff gutter': 90,
'diff added': 32,
'diff removed': 31,
'diff added inline': '30;42',
'diff removed inline': '30;41'
};

And possibly stick even more with the style of eg. durations and resolve to the color in base.js:

mocha/lib/reporters/base.js

Lines 369 to 377 in 24560c1

runner.on(EVENT_TEST_PASS, function (test) {
if (test.duration > test.slow()) {
test.speed = 'slow';
} else if (test.duration > test.slow() / 2) {
test.speed = 'medium';
} else {
test.speed = 'fast';
}
});

And maybe do a similar style and have two levels: 'some retries' / 'many retries' With only the latter resolving to bright yellow

` (retry x${test._currentRetry})`
CheadleCheadle marked this conversation as resolved.
Show resolved Hide resolved
);
return retryFmt;
} else {
return '';
}
}
var fmt;
if (test.speed === 'fast') {
fmt =
indent() +
color('checkmark', ' ' + Base.symbols.ok) +
color('pass', ' %s');
Base.consoleLog(fmt, test.title);
Base.consoleLog(fmt, test.title, logRetries());
} else {
fmt =
indent() +
color('checkmark', ' ' + Base.symbols.ok) +
color('pass', ' %s') +
color(test.speed, ' (%dms)');
Base.consoleLog(fmt, test.title, test.duration);
Base.consoleLog(fmt, test.title, test.duration, logRetries());
}
Comment on lines 91 to 98
Copy link
Member

Choose a reason for hiding this comment

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

I would opt for something like this instead, to keep in the style of the others:

  runner.on(EVENT_TEST_PASS, function (test) {
    var fmt =
      indent() +
      color('checkmark', '  ' + Base.symbols.ok) +
      color('pass', ' %s');
    var fmtVars = [test.title];

    if (test.speed !== 'fast') {
      fmt += color(test.speed, ' (%dms)');
      fmtVars.push(test.duration);
    }

    if (test._currentRetry > 0) {
      fmt += color('bright yellow', ' (retry x%d)');
      fmtVars.push(test._currentRetry);
    }

    Base.consoleLog.apply(Base, [fmt].concat(fmtVars));
  });

});

runner.on(EVENT_TEST_FAIL, function (test) {
runner.on(EVENT_TEST_FAIL, function(test) {
Base.consoleLog(indent() + color('fail', ' %d) %s'), ++n, test.title);
});

Expand Down
63 changes: 48 additions & 15 deletions test/integration/retries.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ var runJSON = helpers.runMochaJSON;
var args = [];
var bang = require('../../lib/reporters/base').symbols.bang;

describe('retries', function () {
it('are ran in correct order', function (done) {
describe('retries', function() {
it('are ran in correct order', function(done) {
helpers.runMocha(
'retries/hooks.fixture.js',
['--reporter', 'dot'],
function (err, res) {
function(err, res) {
var lines, expected;

if (err) {
Expand All @@ -21,10 +21,10 @@ describe('retries', function () {

lines = res.output
.split(helpers.SPLIT_DOT_REPORTER_REGEXP)
.map(function (line) {
.map(function(line) {
return line.trim();
})
.filter(function (line) {
.filter(function(line) {
return line.length;
})
.slice(0, -1);
Expand All @@ -49,7 +49,7 @@ describe('retries', function () {
'after'
];

expected.forEach(function (line, i) {
expected.forEach(function(line, i) {
assert.strictEqual(lines[i], line);
});

Expand All @@ -59,8 +59,8 @@ describe('retries', function () {
);
});

it('should exit early if test passes', function (done) {
runJSON('retries/early-pass.fixture.js', args, function (err, res) {
it('should exit early if test passes', function(done) {
runJSON('retries/early-pass.fixture.js', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -74,8 +74,8 @@ describe('retries', function () {
});
});

it('should let test override', function (done) {
runJSON('retries/nested.fixture.js', args, function (err, res) {
it('should let test override', function(done) {
runJSON('retries/nested.fixture.js', args, function(err, res) {
if (err) {
done(err);
return;
Expand All @@ -89,11 +89,11 @@ describe('retries', function () {
});
});

it('should not hang w/ async test', function (done) {
it('should not hang w/ async test', function(done) {
helpers.runMocha(
'retries/async.fixture.js',
['--reporter', 'dot'],
function (err, res) {
function(err, res) {
var lines, expected;

if (err) {
Expand All @@ -103,10 +103,10 @@ describe('retries', function () {

lines = res.output
.split(helpers.SPLIT_DOT_REPORTER_REGEXP)
.map(function (line) {
.map(function(line) {
return line.trim();
})
.filter(function (line) {
.filter(function(line) {
return line.length;
})
.slice(0, -1);
Expand All @@ -125,7 +125,7 @@ describe('retries', function () {
'after'
];

expected.forEach(function (line, i) {
expected.forEach(function(line, i) {
assert.strictEqual(lines[i], line);
});

Expand All @@ -134,4 +134,37 @@ describe('retries', function () {
}
);
});

it('should return current retry count out of total retry count', function(done) {
helpers.runMocha(
'retries/early-pass.fixture.js',
['--reporter', 'spec'],
function(err, res) {
var lines, expected;

if (err) {
done(err);
return;
}

lines = res.output
.split(helpers.SPLIT_DOT_REPORTER_REGEXP)
.map(function(line) {
return line.trim();
})
.filter(function(line) {
return line.length;
})
.slice(0, -1);

var retryPattern = /\(retry x\d+\)/;

var actual = lines[1].match(retryPattern)[0];
var expected = '(retry x1)';

assert.equal(actual, expected);
done();
}
);
});
});
50 changes: 25 additions & 25 deletions test/reporters/spec.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@ var EVENT_TEST_FAIL = events.EVENT_TEST_FAIL;
var EVENT_TEST_PASS = events.EVENT_TEST_PASS;
var EVENT_TEST_PENDING = events.EVENT_TEST_PENDING;

describe('Spec reporter', function () {
describe('Spec reporter', function() {
var runReporter = makeRunReporter(Spec);
var expectedTitle = 'expectedTitle';
var noop = function () {};
var noop = function() { };

beforeEach(function () {
beforeEach(function() {
sinon.stub(Base, 'useColors').value(false);
});

afterEach(function () {
afterEach(function() {
sinon.restore();
});

describe('event handlers', function () {
describe("on 'suite' event", function () {
it('should return title', function () {
describe('event handlers', function() {
describe("on 'suite' event", function() {
it('should return title', function() {
var suite = {
title: expectedTitle
};
Expand All @@ -42,16 +42,16 @@ describe('Spec reporter', function () {
suite
);
var options = {};
var stdout = runReporter({epilogue: noop}, runner, options);
var stdout = runReporter({ epilogue: noop }, runner, options);
sinon.restore();

var expectedArray = [expectedTitle + '\n'];
expect(stdout, 'to equal', expectedArray);
});
});

describe("on 'pending' event", function () {
it('should return title', function () {
describe("on 'pending' event", function() {
it('should return title', function() {
var suite = {
title: expectedTitle
};
Expand All @@ -63,22 +63,22 @@ describe('Spec reporter', function () {
suite
);
var options = {};
var stdout = runReporter({epilogue: noop}, runner, options);
var stdout = runReporter({ epilogue: noop }, runner, options);
sinon.restore();

var expectedArray = [' - ' + expectedTitle + '\n'];
expect(stdout, 'to equal', expectedArray);
});
});

describe("on 'pass' event", function () {
describe('when test speed is slow', function () {
it('should return expected tick, title, and duration', function () {
describe("on 'pass' event", function() {
describe('when test speed is slow', function() {
it('should return expected tick, title, and duration', function() {
var expectedDuration = 2;
var test = {
title: expectedTitle,
duration: expectedDuration,
slow: function () {
slow: function() {
return 1;
}
};
Expand All @@ -90,7 +90,7 @@ describe('Spec reporter', function () {
test
);
var options = {};
var stdout = runReporter({epilogue: noop}, runner, options);
var stdout = runReporter({ epilogue: noop }, runner, options);
sinon.restore();

var expectedString =
Expand All @@ -101,18 +101,18 @@ describe('Spec reporter', function () {
' (' +
expectedDuration +
'ms)' +
'\n';
' \n';
expect(stdout[0], 'to be', expectedString);
});
});

describe('when test speed is fast', function () {
it('should return expected tick, title without a duration', function () {
describe('when test speed is fast', function() {
it('should return expected tick, title without a duration', function() {
var expectedDuration = 1;
var test = {
title: expectedTitle,
duration: expectedDuration,
slow: function () {
slow: function() {
return 2;
}
};
Expand All @@ -124,18 +124,18 @@ describe('Spec reporter', function () {
test
);
var options = {};
var stdout = runReporter({epilogue: noop}, runner, options);
var stdout = runReporter({ epilogue: noop }, runner, options);
sinon.restore();

var expectedString =
' ' + Base.symbols.ok + ' ' + expectedTitle + '\n';
' ' + Base.symbols.ok + ' ' + expectedTitle + ' \n';
expect(stdout[0], 'to be', expectedString);
});
});
});

describe("on 'fail' event", function () {
it('should return title and function count', function () {
describe("on 'fail' event", function() {
it('should return title and function count', function() {
var functionCount = 1;
var test = {
title: expectedTitle
Expand All @@ -148,7 +148,7 @@ describe('Spec reporter', function () {
test
);
var options = {};
var stdout = runReporter({epilogue: noop}, runner, options);
var stdout = runReporter({ epilogue: noop }, runner, options);
sinon.restore();

var expectedArray = [
Expand Down