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 all 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
Empty file.
18 changes: 16 additions & 2 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 @@ -67,20 +67,34 @@ function Spec(runner, options) {
});

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})`
);
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));
  });

});

Expand Down
33 changes: 33 additions & 0 deletions test/integration/retries.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
);
});
});
4 changes: 2 additions & 2 deletions test/reporters/spec.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Spec reporter', function () {
' (' +
expectedDuration +
'ms)' +
'\n';
' \n';
expect(stdout[0], 'to be', expectedString);
});
});
Expand All @@ -128,7 +128,7 @@ describe('Spec reporter', function () {
sinon.restore();

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