Skip to content

Commit

Permalink
test: migrate test runner message tests to snapshot
Browse files Browse the repository at this point in the history
PR-URL: #47392
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
MoLow authored and RafaelGSS committed Apr 13, 2023
1 parent 54d17ff commit 757a586
Show file tree
Hide file tree
Showing 41 changed files with 460 additions and 331 deletions.
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@

# Test runner

/test/message/test_runner_* @nodejs/test_runner
/test/parallel/test-runner-* @nodejs/test_runner
/doc/api/test.md @nodejs/test_runner
/lib/test.js @nodejs/test_runner
Expand Down
6 changes: 6 additions & 0 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,12 @@ environment variables.
If set, `NODE_COMMON_PORT`'s value overrides the `common.PORT` default value of
12346\.

### `NODE_REGENERATE_SNAPSHOTS`

If set, test snapshots for a the current test are regenerated.
for example `NODE_REGENERATE_SNAPSHOTS=1 out/Release/node test/parallel/test-runner-output.mjs`
will update all the test runner output snapshots.

### `NODE_SKIP_FLAG_CHECK`

If set, command line arguments passed to individual tests are not validated.
Expand Down
53 changes: 53 additions & 0 deletions test/common/assertSnapshot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';
const common = require('.');
const path = require('node:path');
const fs = require('node:fs/promises');
const assert = require('node:assert/strict');


const stackFramesRegexp = /(\s+)((.+?)\s+\()?(?:\(?(.+?):(\d+)(?::(\d+))?)\)?(\s+\{)?(\n|$)/g;
const windowNewlineRegexp = /\r/g;

function replaceStackTrace(str) {
return str.replace(stackFramesRegexp, '$1*$7\n');
}

function replaceWindowsLineEndings(str) {
return str.replace(windowNewlineRegexp, '');
}

function transform(...args) {
return (str) => args.reduce((acc, fn) => fn(acc), str);
}

function getSnapshotPath(filename) {
const { name, dir } = path.parse(filename);
return path.resolve(dir, `${name}.snapshot`);
}

async function assertSnapshot(actual, filename = process.argv[1]) {
const snapshot = getSnapshotPath(filename);
if (process.env.NODE_REGENERATE_SNAPSHOTS) {
await fs.writeFile(snapshot, actual);
} else {
const expected = await fs.readFile(snapshot, 'utf8');
assert.strictEqual(actual, replaceWindowsLineEndings(expected));
}
}

async function spawnAndAssert(filename, transform = (x) => x) {
// TODO: Add an option to this function to alternatively or additionally compare stderr.
// For now, tests that want to check stderr or both stdout and stderr can use spawnPromisified.
const flags = common.parseTestFlags(filename);
const { stdout } = await common.spawnPromisified(process.execPath, [...flags, filename]);
await assertSnapshot(transform(stdout), filename);
}

module.exports = {
assertSnapshot,
getSnapshotPath,
replaceStackTrace,
replaceWindowsLineEndings,
spawnAndAssert,
transform,
};
83 changes: 45 additions & 38 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,31 @@ const hasOpenSSL3 = hasCrypto &&

const hasQuic = hasCrypto && !!process.config.variables.openssl_quic;

function parseTestFlags(filename = process.argv[1]) {
// The copyright notice is relatively big and the flags could come afterwards.
const bytesToRead = 1500;
const buffer = Buffer.allocUnsafe(bytesToRead);
const fd = fs.openSync(filename, 'r');
const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead);
fs.closeSync(fd);
const source = buffer.toString('utf8', 0, bytesRead);

const flagStart = source.indexOf('// Flags: --') + 10;

if (flagStart === 9) {
return [];
}
let flagEnd = source.indexOf('\n', flagStart);
// Normalize different EOL.
if (source[flagEnd - 1] === '\r') {
flagEnd--;
}
return source
.substring(flagStart, flagEnd)
.replace(/_/g, '-')
.split(' ');
}

// Check for flags. Skip this for workers (both, the `cluster` module and
// `worker_threads`) and child processes.
// If the binary was built without-ssl then the crypto flags are
Expand All @@ -71,44 +96,25 @@ if (process.argv.length === 2 &&
hasCrypto &&
require('cluster').isPrimary &&
fs.existsSync(process.argv[1])) {
// The copyright notice is relatively big and the flags could come afterwards.
const bytesToRead = 1500;
const buffer = Buffer.allocUnsafe(bytesToRead);
const fd = fs.openSync(process.argv[1], 'r');
const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead);
fs.closeSync(fd);
const source = buffer.toString('utf8', 0, bytesRead);

const flagStart = source.indexOf('// Flags: --') + 10;
if (flagStart !== 9) {
let flagEnd = source.indexOf('\n', flagStart);
// Normalize different EOL.
if (source[flagEnd - 1] === '\r') {
flagEnd--;
}
const flags = source
.substring(flagStart, flagEnd)
.replace(/_/g, '-')
.split(' ');
const args = process.execArgv.map((arg) => arg.replace(/_/g, '-'));
for (const flag of flags) {
if (!args.includes(flag) &&
// If the binary is build without `intl` the inspect option is
// invalid. The test itself should handle this case.
(process.features.inspector || !flag.startsWith('--inspect'))) {
console.log(
'NOTE: The test started as a child_process using these flags:',
inspect(flags),
'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
);
const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
const options = { encoding: 'utf8', stdio: 'inherit' };
const result = spawnSync(process.execPath, args, options);
if (result.signal) {
process.kill(0, result.signal);
} else {
process.exit(result.status);
}
const flags = parseTestFlags();
const args = process.execArgv.map((arg) => arg.replace(/_/g, '-'));
for (const flag of flags) {
if (!args.includes(flag) &&
// If the binary is build without `intl` the inspect option is
// invalid. The test itself should handle this case.
(process.features.inspector || !flag.startsWith('--inspect'))) {
console.log(
'NOTE: The test started as a child_process using these flags:',
inspect(flags),
'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
);
const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
const options = { encoding: 'utf8', stdio: 'inherit' };
const result = spawnSync(process.execPath, args, options);
if (result.signal) {
process.kill(0, result.signal);
} else {
process.exit(result.status);
}
}
}
Expand Down Expand Up @@ -929,6 +935,7 @@ const common = {
mustSucceed,
nodeProcessAborted,
PIPE,
parseTestFlags,
platformTimeout,
printSkipMessage,
pwdCommand,
Expand Down
2 changes: 2 additions & 0 deletions test/common/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const {
getCallSite,
mustNotCall,
mustNotMutateObjectDeep,
parseTestFlags,
printSkipMessage,
skip,
nodeProcessAborted,
Expand Down Expand Up @@ -88,6 +89,7 @@ export {
getCallSite,
mustNotCall,
mustNotMutateObjectDeep,
parseTestFlags,
printSkipMessage,
skip,
nodeProcessAborted,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --no-warnings
'use strict';
require('../common');
require('../../../common');
const test = require('node:test');

test('promise timeout signal', { signal: AbortSignal.timeout(1) }, async (t) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ TAP version 13
# Subtest: not ok 2
not ok 6 - not ok 2
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
...
# Subtest: not ok 3
not ok 7 - not ok 3
---
duration_ms: *
duration_ms: ZERO
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
Expand All @@ -59,7 +59,7 @@ TAP version 13
# Subtest: not ok 4
not ok 8 - not ok 4
---
duration_ms: *
duration_ms: ZERO
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
Expand All @@ -79,7 +79,7 @@ TAP version 13
# Subtest: not ok 5
not ok 9 - not ok 5
---
duration_ms: *
duration_ms: ZERO
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
Expand Down Expand Up @@ -161,15 +161,15 @@ not ok 2 - promise abort signal
# Subtest: not ok 2
not ok 6 - not ok 2
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
...
# Subtest: not ok 3
not ok 7 - not ok 3
---
duration_ms: *
duration_ms: ZERO
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
Expand All @@ -189,7 +189,7 @@ not ok 2 - promise abort signal
# Subtest: not ok 4
not ok 8 - not ok 4
---
duration_ms: *
duration_ms: ZERO
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
Expand All @@ -209,7 +209,7 @@ not ok 2 - promise abort signal
# Subtest: not ok 5
not ok 9 - not ok 5
---
duration_ms: *
duration_ms: ZERO
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --no-warnings
'use strict';
require('../common');
require('../../../common');
const { describe, it } = require('node:test');

describe('describe timeout signal', { signal: AbortSignal.timeout(1) }, (t) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,31 @@ TAP version 13
# Subtest: not ok 2
not ok 6 - not ok 2
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
...
# Subtest: not ok 3
not ok 7 - not ok 3
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
...
# Subtest: not ok 4
not ok 8 - not ok 4
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
...
# Subtest: not ok 5
not ok 9 - not ok 5
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --no-warnings
'use strict';
require('../common');
require('../../../common');
const assert = require('node:assert');
const { describe, it, test } = require('node:test');
const util = require('util');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,10 @@ ok 21 - immediate resolve pass
*
*
*
new Promise (<anonymous>)
*
*
*
*
Array.map (<anonymous>)
...
# Subtest: mixing describe/it and test should work
ok 2 - mixing describe/it and test should work
Expand Down Expand Up @@ -469,10 +469,10 @@ not ok 53 - custom inspect symbol that throws fail
*
*
*
new Promise (<anonymous>)
*
*
*
*
Array.map (<anonymous>)
...
# Subtest: sync throw fails at second
not ok 2 - sync throw fails at second
Expand All @@ -491,7 +491,7 @@ not ok 53 - custom inspect symbol that throws fail
*
*
*
*
async Promise.all (index 0)
...
1..2
not ok 54 - subtest sync throw fails
Expand All @@ -506,7 +506,7 @@ not ok 54 - subtest sync throw fails
# Subtest: should not run
not ok 1 - should not run
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
Expand Down Expand Up @@ -535,7 +535,7 @@ not ok 55 - describe sync throw fails
# Subtest: should not run
not ok 1 - should not run
---
duration_ms: *
duration_ms: ZERO
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --no-warnings
'use strict';
require('../common');
require('../../../common');
const { describe, it } = require('node:test');

describe('nested - no tests', () => {
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/test-runner/output/dot_reporter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Flags: --no-warnings
'use strict';
require('../../../common');
const fixtures = require('../../../common/fixtures');
const spawn = require('node:child_process').spawn;

spawn(process.execPath,
['--no-warnings', '--test-reporter', 'dot', fixtures.path('test-runner/output/output.js')], { stdio: 'inherit' });

0 comments on commit 757a586

Please sign in to comment.