From d09e63701eaf8991117e072572b74c8f16bb6af5 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 6 Oct 2018 18:27:28 -0700 Subject: [PATCH 1/3] test: remove internal errorCache property The internal `assert` modules `errorCache` property is exposed only for testing. The one test that used it is rewritten here to not use it. This has the following advantages: * The test now makes sure that there is an empty cache in a more robust way. Instead of relying on the internal implementation of `errorCache`, it simply spawns a separate process. * One less test using the `--expose-internals` flag. --- ...ssert-builtins-not-read-from-filesystem.js | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/test/parallel/test-assert-builtins-not-read-from-filesystem.js b/test/parallel/test-assert-builtins-not-read-from-filesystem.js index 000798aca267a3..7855f830add10b 100644 --- a/test/parallel/test-assert-builtins-not-read-from-filesystem.js +++ b/test/parallel/test-assert-builtins-not-read-from-filesystem.js @@ -5,47 +5,44 @@ require('../common'); const assert = require('assert'); +const EventEmitter = require('events'); +const e = new EventEmitter(); +e.on('hello', assert); if (process.argv[2] !== 'child') { const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); const { spawnSync } = require('child_process'); - const { output, status, error } = - spawnSync(process.execPath, - ['--expose-internals', process.argv[1], 'child'], - { cwd: tmpdir.path, env: process.env }); - assert.ifError(error); - assert.strictEqual(status, 0, `Exit code: ${status}\n${output}`); -} else { - const EventEmitter = require('events'); - const { errorCache } = require('internal/assert'); - const { writeFileSync } = require('fs'); - const e = new EventEmitter(); - - e.on('hello', assert); let threw = false; try { e.emit('hello', false); } catch (err) { const frames = err.stack.split('\n'); - const [, filename, line, column] = frames[1].match(/\((.+):(\d+):(\d+)\)/); - // Reset the cache to check again - const size = errorCache.size; - errorCache.delete(`${filename}${line - 1}${column - 1}`); - assert.strictEqual(errorCache.size, size - 1); - const data = `${'\n'.repeat(line - 1)}${' '.repeat(column - 1)}` + - 'ok(failed(badly));'; + const [, filename, , ] = frames[1].match(/\((.+):(\d+):(\d+)\)/); + // Spawn a child process to avoid the error having been cached in the assert + // module's `errorCache` Map. - writeFileSync(filename, data); - assert.throws( - () => e.emit('hello', false), - { - message: 'false == true' - } - ); + const { output, status, error } = + spawnSync(process.execPath, + [process.argv[1], 'child', filename], + { cwd: tmpdir.path, env: process.env }); + assert.ifError(error); + assert.strictEqual(status, 0, `Exit code: ${status}\n${output}`); threw = true; - } - assert(threw); + assert.ok(threw); +} else { + const { writeFileSync } = require('fs'); + const [, , , filename, line, column] = process.argv; + const data = `${'\n'.repeat(line - 1)}${' '.repeat(column - 1)}` + + 'ok(failed(badly));'; + + writeFileSync(filename, data); + assert.throws( + () => e.emit('hello', false), + { + message: 'false == true' + } + ); } From cd2e66a1a4064d9255745cb8b3940d80a43c7524 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 6 Oct 2018 18:27:54 -0700 Subject: [PATCH 2/3] assert: remove internal errorCache property The internal assert module exposed an errorCache property solely for testing. It is no longer necessary. Remove it. --- lib/assert.js | 4 +++- lib/internal/assert.js | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 0f3d770358f7c6..9990d0e888e1c5 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -27,12 +27,14 @@ const { codes: { ERR_INVALID_ARG_VALUE, ERR_INVALID_RETURN_VALUE } } = require('internal/errors'); -const { AssertionError, errorCache } = require('internal/assert'); +const { AssertionError } = require('internal/assert'); const { openSync, closeSync, readSync } = require('fs'); const { inspect, types: { isPromise, isRegExp } } = require('util'); const { EOL } = require('internal/constants'); const { NativeModule } = require('internal/bootstrap/loaders'); +const errorCache = new Map(); + let isDeepEqual; let isDeepStrictEqual; let parseExpressionAt; diff --git a/lib/internal/assert.js b/lib/internal/assert.js index e174cddb60a4e3..829f6663191dff 100644 --- a/lib/internal/assert.js +++ b/lib/internal/assert.js @@ -396,6 +396,5 @@ class AssertionError extends Error { } module.exports = { - AssertionError, - errorCache: new Map() + AssertionError }; From 15993e50222aec7ff6c16634d9499774096c52cb Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 8 Oct 2018 07:27:04 -0700 Subject: [PATCH 3/3] fixup! assert: remove internal errorCache property --- .../parallel/test-assert-builtins-not-read-from-filesystem.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-assert-builtins-not-read-from-filesystem.js b/test/parallel/test-assert-builtins-not-read-from-filesystem.js index 7855f830add10b..7a713a2ea432c1 100644 --- a/test/parallel/test-assert-builtins-not-read-from-filesystem.js +++ b/test/parallel/test-assert-builtins-not-read-from-filesystem.js @@ -19,13 +19,13 @@ if (process.argv[2] !== 'child') { e.emit('hello', false); } catch (err) { const frames = err.stack.split('\n'); - const [, filename, , ] = frames[1].match(/\((.+):(\d+):(\d+)\)/); + const [, filename, line, column] = frames[1].match(/\((.+):(\d+):(\d+)\)/); // Spawn a child process to avoid the error having been cached in the assert // module's `errorCache` Map. const { output, status, error } = spawnSync(process.execPath, - [process.argv[1], 'child', filename], + [process.argv[1], 'child', filename, line, column], { cwd: tmpdir.path, env: process.env }); assert.ifError(error); assert.strictEqual(status, 0, `Exit code: ${status}\n${output}`);