Skip to content

Commit

Permalink
Improve test result accessibility
Browse files Browse the repository at this point in the history 
* Explain diff gutter symbols. Fixes #1558
* Add test result labels. Color covers figure and label, and indicates passed-as-expected vs. not (expected/unexpected fail or unexpected pass). Fixes #2919
* Bold the source line of errors in code excerpts
* TAP reporter: Strip ANSI control sequences from error labels
* Print assertion error in italics, to better differentiate from the test title.

Co-authored-by: Mark Wubben <mark@novemberborn.net>
  • Loading branch information
@gibson042 @novemberborn
gibson042 and novemberborn committed Sep 4, 2022
1 parent cc10b0c commit 797287a
Show file tree
Hide file tree
Showing 23 changed files with 169 additions and 152 deletions.
4 changes: 3 additions & 1 deletion lib/assert.js
View file
Expand Up @@ -8,8 +8,10 @@ import {SnapshotError, VersionMismatchError} from './snapshot-manager.js';

function formatDescriptorDiff(actualDescriptor, expectedDescriptor, options) {
options = {...options, ...concordanceOptions};
const {diffGutters} = options.theme;
const {insertLine, deleteLine} = options.theme.string.diff;
return {
label: 'Difference:',
label: `Difference (${diffGutters.actual}${deleteLine.open}actual${deleteLine.close}, ${diffGutters.expected}${insertLine.open}expected${insertLine.close}):`,
formatted: concordance.diffDescriptors(actualDescriptor, expectedDescriptor, options),
};
}
Expand Down
2 changes: 1 addition & 1 deletion lib/code-excerpt.js
View file
Expand Up @@ -43,7 +43,7 @@ export default function exceptCode(source, options = {}) {
const coloredLineNumber = isErrorSource ? lineNumber : chalk.grey(lineNumber);
const result = ` ${coloredLineNumber} ${item.value.padEnd(extendedWidth)}`;

return isErrorSource ? chalk.bgRed(result) : result;
return isErrorSource ? chalk.bgRed.bold(result) : result;
})
.join('\n');
}
2 changes: 1 addition & 1 deletion lib/concordance-options.js
View file
Expand Up @@ -85,7 +85,7 @@ const colorTheme = {
undefined: ansiStyles.yellow,
};

const plainTheme = JSON.parse(JSON.stringify(colorTheme), value => typeof value === 'string' ? stripAnsi(value) : value);
const plainTheme = JSON.parse(JSON.stringify(colorTheme), (_name, value) => typeof value === 'string' ? stripAnsi(value) : value);

const theme = chalk.level > 0 ? colorTheme : plainTheme;

Expand Down
27 changes: 21 additions & 6 deletions lib/reporters/default.js
View file
Expand Up @@ -254,9 +254,9 @@ export default class Reporter {

case 'selected-test': {
if (event.skip) {
this.lineWriter.writeLine(colors.skip(`- ${this.prefixTitle(event.testFile, event.title)}`));
this.lineWriter.writeLine(colors.skip(`- [skip] ${this.prefixTitle(event.testFile, event.title)}`));
} else if (event.todo) {
this.lineWriter.writeLine(colors.todo(`- ${this.prefixTitle(event.testFile, event.title)}`));
this.lineWriter.writeLine(colors.todo(`- [todo] ${this.prefixTitle(event.testFile, event.title)}`));
}

break;
Expand Down Expand Up @@ -514,15 +514,30 @@ export default class Reporter {
}

writeTestSummary(event) {
// Prefix icon indicates matched expectations vs. not.
// Prefix color indicates passed-as-expected vs. not (fail or unexpected pass).
// This yields four possibilities, which in the standard configuration render as:
// * normal test, pass: <green>✔</green>
// * normal test, fail: <red>✘ [fail]</red>
// * fail-expected test, fail: <red>✔ [expected fail]</red>
// * fail-expected test, pass: <red>✘ [unexpected pass]</red>
let prefix;
let suffix;
if (event.type === 'hook-failed' || event.type === 'test-failed') {
this.write(`${colors.error(figures.cross)} ${this.prefixTitle(event.testFile, event.title)} ${colors.error(event.err.message)}`);
const type = event.knownFailing ? '[unexpected pass]' : '[fail]';
prefix = colors.error(`${figures.cross} ${type}:`);
suffix = chalk.italic(colors.error(event.err.message));
} else if (event.knownFailing) {
this.write(`${colors.error(figures.tick)} ${colors.error(this.prefixTitle(event.testFile, event.title))}`);
prefix = colors.error(figures.tick + ' [expected fail]');
} else {
const duration = event.duration > this.durationThreshold ? colors.duration(' (' + prettyMs(event.duration) + ')') : '';
this.write(`${colors.pass(figures.tick)} ${this.prefixTitle(event.testFile, event.title)}${duration}`);
prefix = colors.pass(figures.tick);
if (event.duration > this.durationThreshold) {
suffix = colors.duration(`(${prettyMs(event.duration)})`);
}
}

const label = this.prefixTitle(event.testFile, event.title);
this.write(`${prefix} ${label}${suffix ? ' ' + suffix : ''}`);
this.writeLogs(event);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/tap.js
View file
Expand Up @@ -29,7 +29,7 @@ function dumpError(error) {
}

if (error.values.length > 0) {
object.values = Object.fromEntries(error.values.map(({label, formatted}) => [label, stripAnsi(formatted)]));
object.values = Object.fromEntries(error.values.map(({label, formatted}) => [stripAnsi(label), stripAnsi(formatted)]));
}
}

Expand Down
24 changes: 12 additions & 12 deletions test-tap/assert.js
View file
Expand Up @@ -56,7 +56,7 @@ function assertFailure(t, subset) {
if (subset.values) {
t.equal(lastFailure.values.length, subset.values.length);
for (const [i, s] of lastFailure.values.entries()) {
t.equal(s.label, subset.values[i].label);
t.equal(stripAnsi(s.label), subset.values[i].label);
t.match(stripAnsi(s.formatted), subset.values[i].formatted);
}
} else {
Expand Down Expand Up @@ -279,7 +279,7 @@ test('.is()', t => {
message: '',
raw: {actual: 'foo', expected: 'bar'},
values: [
{label: 'Difference:', formatted: /- 'foo'\n\+ 'bar'/},
{label: 'Difference (- actual, + expected):', formatted: /- 'foo'\n\+ 'bar'/},
],
});

Expand All @@ -289,31 +289,31 @@ test('.is()', t => {
expected: 42,
message: '',
values: [
{label: 'Difference:', formatted: /- 'foo'\n\+ 42/},
{label: 'Difference (- actual, + expected):', formatted: /- 'foo'\n\+ 42/},
],
});

failsWith(t, () => assertions.is('foo', 42, 'my message'), {
assertion: 'is',
message: 'my message',
values: [
{label: 'Difference:', formatted: /- 'foo'\n\+ 42/},
{label: 'Difference (- actual, + expected):', formatted: /- 'foo'\n\+ 42/},
],
});

failsWith(t, () => assertions.is(0, -0, 'my message'), {
assertion: 'is',
message: 'my message',
values: [
{label: 'Difference:', formatted: /- 0\n\+ -0/},
{label: 'Difference (- actual, + expected):', formatted: /- 0\n\+ -0/},
],
});

failsWith(t, () => assertions.is(-0, 0, 'my message'), {
assertion: 'is',
message: 'my message',
values: [
{label: 'Difference:', formatted: /- -0\n\+ 0/},
{label: 'Difference (- actual, + expected):', formatted: /- -0\n\+ 0/},
],
});

Expand Down Expand Up @@ -535,20 +535,20 @@ test('.deepEqual()', t => {
assertion: 'deepEqual',
message: '',
raw: {actual: 'foo', expected: 'bar'},
values: [{label: 'Difference:', formatted: /- 'foo'\n\+ 'bar'/}],
values: [{label: 'Difference (- actual, + expected):', formatted: /- 'foo'\n\+ 'bar'/}],
});

failsWith(t, () => assertions.deepEqual('foo', 42), {
assertion: 'deepEqual',
message: '',
raw: {actual: 'foo', expected: 42},
values: [{label: 'Difference:', formatted: /- 'foo'\n\+ 42/}],
values: [{label: 'Difference (- actual, + expected):', formatted: /- 'foo'\n\+ 42/}],
});

failsWith(t, () => assertions.deepEqual('foo', 42, 'my message'), {
assertion: 'deepEqual',
message: 'my message',
values: [{label: 'Difference:', formatted: /- 'foo'\n\+ 42/}],
values: [{label: 'Difference (- actual, + expected):', formatted: /- 'foo'\n\+ 42/}],
});

failsWith(t, () => assertions.deepEqual({}, {}, null), {
Expand Down Expand Up @@ -758,7 +758,7 @@ test('.like()', t => {
failsWith(t, () => assertions.like({a: 'foo', b: 'irrelevant'}, {a: 'bar'}), {
assertion: 'like',
message: '',
values: [{label: 'Difference:', formatted: /{\n-\s*a: 'foo',\n\+\s*a: 'bar',\n\s*}/}],
values: [{label: 'Difference (- actual, + expected):', formatted: /{\n-\s*a: 'foo',\n\+\s*a: 'bar',\n\s*}/}],
});

t.end();
Expand Down Expand Up @@ -1429,7 +1429,7 @@ test('.snapshot()', async t => {
failsWith(t, () => assertions.snapshot({foo: 'not bar'}), {
assertion: 'snapshot',
message: 'Did not match snapshot',
values: [{label: 'Difference:', formatted: ' {\n- foo: \'not bar\',\n+ foo: \'bar\',\n }'}],
values: [{label: 'Difference (- actual, + expected):', formatted: ' {\n- foo: \'not bar\',\n+ foo: \'bar\',\n }'}],
});
}

Expand All @@ -1442,7 +1442,7 @@ test('.snapshot()', async t => {
failsWith(t, () => assertions.snapshot({foo: 'not bar'}, 'my message'), {
assertion: 'snapshot',
message: 'my message',
values: [{label: 'Difference:', formatted: ' {\n- foo: \'not bar\',\n+ foo: \'bar\',\n }'}],
values: [{label: 'Difference (- actual, + expected):', formatted: ' {\n- foo: \'not bar\',\n+ foo: \'bar\',\n }'}],
});
}

Expand Down
6 changes: 3 additions & 3 deletions test-tap/code-excerpt.js
View file
Expand Up @@ -22,7 +22,7 @@ test('read code excerpt', t => {
const excerpt = codeExcerpt({file, line: 2, isWithinProject: true, isDependency: false});
const expected = [
` ${chalk.grey('1:')} function a() {`,
chalk.bgRed(' 2: alert(); '),
chalk.bgRed.bold(' 2: alert(); '),
` ${chalk.grey('3:')} } `,
].join('\n');

Expand All @@ -40,7 +40,7 @@ test('truncate lines', t => {
const excerpt = codeExcerpt({file, line: 2, isWithinProject: true, isDependency: false}, {maxWidth: 14});
const expected = [
` ${chalk.grey('1:')} functio…`,
chalk.bgRed(' 2: alert…'),
chalk.bgRed.bold(' 2: alert…'),
` ${chalk.grey('3:')} } `,
].join('\n');

Expand All @@ -66,7 +66,7 @@ test('format line numbers', t => {
const excerpt = codeExcerpt({file, line: 10, isWithinProject: true, isDependency: false});
const expected = [
` ${chalk.grey(' 9:')} function a() {`,
chalk.bgRed(' 10: alert(); '),
chalk.bgRed.bold(' 10: alert(); '),
` ${chalk.grey('11:')} } `,
].join('\n');

Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.edgecases.v14.log
View file
Expand Up @@ -20,7 +20,7 @@
import-and-use-test-member.cjs:3

2:
 3: test('pass', t => t.pass());
 3: test('pass', t => t.pass());
4:

TypeError: test is not a function
Expand All @@ -39,7 +39,7 @@

throws.cjs:1

 1: throw new Error('throws');
 1: throw new Error('throws');
2:

Error: throws
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.edgecases.v16.log
View file
Expand Up @@ -20,7 +20,7 @@
import-and-use-test-member.cjs:3

2:
 3: test('pass', t => t.pass());
 3: test('pass', t => t.pass());
4:

TypeError: test is not a function
Expand All @@ -39,7 +39,7 @@

throws.cjs:1

 1: throw new Error('throws');
 1: throw new Error('throws');
2:

Error: throws
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.edgecases.v18.log
View file
Expand Up @@ -24,7 +24,7 @@
import-and-use-test-member.cjs:3

2:
 3: test('pass', t => t.pass());
 3: test('pass', t => t.pass());
4:

TypeError: test is not a function
Expand All @@ -43,7 +43,7 @@

throws.cjs:1

 1: throw new Error('throws');
 1: throw new Error('throws');
2:

Error: throws
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.failfast.v14.log
View file
@@ -1,6 +1,6 @@

---tty-stream-chunk-separator
✘ a › fails Test failed via `t.fail()`
✘ [fail]: a › fails Test failed via `t.fail()`
---tty-stream-chunk-separator
─

Expand All @@ -9,7 +9,7 @@
a.cjs:3

2:
 3: test('fails', t => t.fail());
 3: test('fails', t => t.fail());
4:

Test failed via `t.fail()`
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.failfast.v16.log
View file
@@ -1,6 +1,6 @@

---tty-stream-chunk-separator
✘ a › fails Test failed via `t.fail()`
✘ [fail]: a › fails Test failed via `t.fail()`
---tty-stream-chunk-separator
─

Expand All @@ -9,7 +9,7 @@
a.cjs:3

2:
 3: test('fails', t => t.fail());
 3: test('fails', t => t.fail());
4:

Test failed via `t.fail()`
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.failfast.v18.log
View file
@@ -1,6 +1,6 @@

---tty-stream-chunk-separator
✘ a › fails Test failed via `t.fail()`
✘ [fail]: a › fails Test failed via `t.fail()`
---tty-stream-chunk-separator
─

Expand All @@ -9,7 +9,7 @@
a.cjs:3

2:
 3: test('fails', t => t.fail());
 3: test('fails', t => t.fail());
4:

Test failed via `t.fail()`
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.failfast2.v14.log
View file
@@ -1,6 +1,6 @@

---tty-stream-chunk-separator
✘ a › fails Test failed via `t.fail()`
✘ [fail]: a › fails Test failed via `t.fail()`
---tty-stream-chunk-separator
─

Expand All @@ -9,7 +9,7 @@
a.cjs:3

2:
 3: test('fails', t => t.fail()); 
 3: test('fails', t => t.fail()); 
4: test('passes', t => t.pass());

Test failed via `t.fail()`
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.failfast2.v16.log
View file
@@ -1,6 +1,6 @@

---tty-stream-chunk-separator
✘ a › fails Test failed via `t.fail()`
✘ [fail]: a › fails Test failed via `t.fail()`
---tty-stream-chunk-separator
─

Expand All @@ -9,7 +9,7 @@
a.cjs:3

2:
 3: test('fails', t => t.fail()); 
 3: test('fails', t => t.fail()); 
4: test('passes', t => t.pass());

Test failed via `t.fail()`
Expand Down
4 changes: 2 additions & 2 deletions test-tap/reporters/default.failfast2.v18.log
View file
@@ -1,6 +1,6 @@

---tty-stream-chunk-separator
✘ a › fails Test failed via `t.fail()`
✘ [fail]: a › fails Test failed via `t.fail()`
---tty-stream-chunk-separator
─

Expand All @@ -9,7 +9,7 @@
a.cjs:3

2:
 3: test('fails', t => t.fail()); 
 3: test('fails', t => t.fail()); 
4: test('passes', t => t.pass());

Test failed via `t.fail()`
Expand Down

0 comments on commit 797287a

Please sign in to comment.