From e8b6c183d46c41b2992c4f5cafceb7f9c6f8710d Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sun, 30 Mar 2025 17:33:58 +0100 Subject: [PATCH 1/4] test: add tests for REPL custom evals --- lib/repl.js | 1 - test/parallel/test-repl-custom-eval.js | 132 +++++++++++++++++++++++++ test/parallel/test-repl-eval.js | 33 ------- 3 files changed, 132 insertions(+), 34 deletions(-) create mode 100644 test/parallel/test-repl-custom-eval.js delete mode 100644 test/parallel/test-repl-eval.js diff --git a/lib/repl.js b/lib/repl.js index e61a24bb041c7e..4cdd7f8db08233 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -302,7 +302,6 @@ function REPLServer(prompt, options.useColors = shouldColorize(options.output); } - // TODO(devsnek): Add a test case for custom eval functions. const preview = options.terminal && (options.preview !== undefined ? !!options.preview : !eval_); diff --git a/test/parallel/test-repl-custom-eval.js b/test/parallel/test-repl-custom-eval.js new file mode 100644 index 00000000000000..20bbdae5f8b9a3 --- /dev/null +++ b/test/parallel/test-repl-custom-eval.js @@ -0,0 +1,132 @@ +'use strict'; + +require('../common'); +const ArrayStream = require('../common/arraystream'); +const assert = require('assert'); +const { describe, it } = require('node:test'); + +const repl = require('repl'); + +describe('repl with custom eval', () => { + it('uses the custom eval function as expected', () => { + const output = getReplOutput('Convert this to upper case', { + terminal: true, + eval: (code, _ctx, _replRes, cb) => cb(null, code.toUpperCase()), + }); + assert.match( + output, + /Convert this to upper case\r\n'CONVERT THIS TO UPPER CASE\\n'/ + ); + }); + + it('surfaces errors as expected', () => { + const output = getReplOutput('Convert this to upper case', { + terminal: true, + eval: (_code, _ctx, _replRes, cb) => cb(new Error('Testing Error')), + }); + assert.match(output, /Uncaught Error: Testing Error\n/); + }); + + it('provides a repl context to the eval callback', async () => { + const context = await new Promise((resolve) => { + const r = repl.start({ + eval: (_cmd, context) => resolve(context), + }); + r.context = { foo: 'bar' }; + r.write('\n.exit\n'); + }); + assert.strictEqual(context.foo, 'bar'); + }); + + it('provides a global context to the eval callback', async () => { + const context = await new Promise((resolve) => { + const r = repl.start({ + useGlobal: true, + eval: (_cmd, context) => resolve(context), + }); + global.foo = 'global_bar'; + r.write('\n.exit\n'); + }); + + assert.strictEqual(context.foo, 'global_bar'); + delete global.foo; + }); + + it('does not access the global context if `useGlobal` is false', async () => { + const context = await new Promise((resolve) => { + const r = repl.start({ + useGlobal: false, + eval: (_cmd, context) => resolve(context), + }); + global.foo = 'global_bar'; + r.write('\n.exit\n'); + }); + + assert.notStrictEqual(context.foo, 'global_bar'); + delete global.foo; + }); + + /** + * Default preprocessor transforms + * function f() {} to + * var f = function f() {} + * This test ensures that original input is preserved. + * Reference: https://github.com/nodejs/node/issues/9743 + */ + it('preserves the original input', async () => { + const cmd = await new Promise((resolve) => { + const r = repl.start({ + eval: (cmd) => resolve(cmd), + }); + r.write('function f() {}\n.exit\n'); + }); + assert.strictEqual(cmd, 'function f() {}\n'); + }); + + it("doesn't show previews by default", () => { + const input = "'Hello custom' + ' eval World!'"; + const output = getReplOutput(input, { + terminal: true, + eval: (code, _ctx, _replRes, cb) => cb(null, eval(code)), + }, false); + assert.strictEqual(output, input); + assert.doesNotMatch(output, /Hello custom eval World!/); + }); + + it('does show previews if `preview` is set to `true`', () => { + const input = "'Hello custom' + ' eval World!'"; + const output = getReplOutput(input, { + terminal: true, + eval: (code, _ctx, _replRes, cb) => cb(null, eval(code)), + preview: true, + }, false); + + const escapedInput = input.replace(/\+/g, '\\+'); + assert.match( + output, + new RegExp(`${escapedInput}\n// 'Hello custom eval World!'`) + ); + }); +}); + +function getReplOutput(input, replOptions, run = true) { + const inputStream = new ArrayStream(); + const outputStream = new ArrayStream(); + + repl.start({ + input: inputStream, + output: outputStream, + ...replOptions, + }); + + let output = ''; + outputStream.write = (chunk) => (output += chunk); + + inputStream.emit('data', input); + + if (run) { + inputStream.run(['']); + } + + return output; +} diff --git a/test/parallel/test-repl-eval.js b/test/parallel/test-repl-eval.js deleted file mode 100644 index d775423fb74a52..00000000000000 --- a/test/parallel/test-repl-eval.js +++ /dev/null @@ -1,33 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const repl = require('repl'); - -{ - let evalCalledWithExpectedArgs = false; - - const options = { - eval: common.mustCall((cmd, context) => { - // Assertions here will not cause the test to exit with an error code - // so set a boolean that is checked later instead. - evalCalledWithExpectedArgs = (cmd === 'function f() {}\n' && - context.foo === 'bar'); - }) - }; - - const r = repl.start(options); - r.context = { foo: 'bar' }; - - try { - // Default preprocessor transforms - // function f() {} to - // var f = function f() {} - // Test to ensure that original input is preserved. - // Reference: https://github.com/nodejs/node/issues/9743 - r.write('function f() {}\n'); - } finally { - r.write('.exit\n'); - } - - assert(evalCalledWithExpectedArgs); -} From afd03ec6116106e6fe0c8837edd3076f8ea05bb8 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Mon, 31 Mar 2025 13:56:42 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/parallel/test-repl-custom-eval.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-repl-custom-eval.js b/test/parallel/test-repl-custom-eval.js index 20bbdae5f8b9a3..21d148adcd78ff 100644 --- a/test/parallel/test-repl-custom-eval.js +++ b/test/parallel/test-repl-custom-eval.js @@ -101,7 +101,7 @@ describe('repl with custom eval', () => { preview: true, }, false); - const escapedInput = input.replace(/\+/g, '\\+'); + const escapedInput = input.replace(/\+/g, '\\+'); // TODO: migrate to `RegExp.escape` when it's available. assert.match( output, new RegExp(`${escapedInput}\n// 'Hello custom eval World!'`) From 4da7e259592cd2c44b318ba465a82252b0c8cb87 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Mon, 31 Mar 2025 19:43:07 +0100 Subject: [PATCH 3/4] move `getReplOutput` up --- test/parallel/test-repl-custom-eval.js | 44 +++++++++++++------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/test/parallel/test-repl-custom-eval.js b/test/parallel/test-repl-custom-eval.js index 21d148adcd78ff..c0533ea234f6ee 100644 --- a/test/parallel/test-repl-custom-eval.js +++ b/test/parallel/test-repl-custom-eval.js @@ -7,6 +7,28 @@ const { describe, it } = require('node:test'); const repl = require('repl'); +function getReplOutput(input, replOptions, run = true) { + const inputStream = new ArrayStream(); + const outputStream = new ArrayStream(); + + repl.start({ + input: inputStream, + output: outputStream, + ...replOptions, + }); + + let output = ''; + outputStream.write = (chunk) => (output += chunk); + + inputStream.emit('data', input); + + if (run) { + inputStream.run(['']); + } + + return output; +} + describe('repl with custom eval', () => { it('uses the custom eval function as expected', () => { const output = getReplOutput('Convert this to upper case', { @@ -108,25 +130,3 @@ describe('repl with custom eval', () => { ); }); }); - -function getReplOutput(input, replOptions, run = true) { - const inputStream = new ArrayStream(); - const outputStream = new ArrayStream(); - - repl.start({ - input: inputStream, - output: outputStream, - ...replOptions, - }); - - let output = ''; - outputStream.write = (chunk) => (output += chunk); - - inputStream.emit('data', input); - - if (run) { - inputStream.run(['']); - } - - return output; -} From 63034cb9808b0abbb564ee856dedeba828b9a096 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sat, 5 Apr 2025 20:58:36 +0100 Subject: [PATCH 4/4] use `concurrency: true` and adjust tests --- test/parallel/test-repl-custom-eval.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-repl-custom-eval.js b/test/parallel/test-repl-custom-eval.js index c0533ea234f6ee..0febf3c9bbfeac 100644 --- a/test/parallel/test-repl-custom-eval.js +++ b/test/parallel/test-repl-custom-eval.js @@ -29,7 +29,7 @@ function getReplOutput(input, replOptions, run = true) { return output; } -describe('repl with custom eval', () => { +describe('repl with custom eval', { concurrency: true }, () => { it('uses the custom eval function as expected', () => { const output = getReplOutput('Convert this to upper case', { terminal: true, @@ -60,32 +60,35 @@ describe('repl with custom eval', () => { assert.strictEqual(context.foo, 'bar'); }); - it('provides a global context to the eval callback', async () => { + it('provides the global context to the eval callback', async () => { const context = await new Promise((resolve) => { const r = repl.start({ useGlobal: true, eval: (_cmd, context) => resolve(context), }); - global.foo = 'global_bar'; + global.foo = 'global_foo'; r.write('\n.exit\n'); }); - assert.strictEqual(context.foo, 'global_bar'); + assert.strictEqual(context.foo, 'global_foo'); delete global.foo; }); - it('does not access the global context if `useGlobal` is false', async () => { + it('inherits variables from the global context but does not use it afterwords if `useGlobal` is false', async () => { + global.bar = 'global_bar'; const context = await new Promise((resolve) => { const r = repl.start({ useGlobal: false, eval: (_cmd, context) => resolve(context), }); - global.foo = 'global_bar'; + global.baz = 'global_baz'; r.write('\n.exit\n'); }); - assert.notStrictEqual(context.foo, 'global_bar'); - delete global.foo; + assert.strictEqual(context.bar, 'global_bar'); + assert.notStrictEqual(context.baz, 'global_baz'); + delete global.bar; + delete global.baz; }); /**