Repository: nodejs/node
Pull request: test: add tests for REPL custom evals (second attempt)

this PR reintroduces the REPL custom eval tests that have been introduced in https://github.com/nodejs/node/pull/57691 but reverted in https://github.com/nodejs/node/pull/57793

the tests were working fine locally but caused failures in CI (like [this one](https://ci.nodejs.org/job/node-test-commit-linux-containered/49958/nodes=ubuntu2204_sharedlibs_withoutssl_x64/testReport/(root)/parallel/test_repl_custom_eval/)) the problem, I believe, being that the `getReplOutput` would return the `output` value right away even though the repl processing is asynchronous. Some small delay there is, I imagine, present in the CI runs and that's what I think was causing the issue.

I addressed this problem by returning a promise that resolves to the repl's output using the repl prompt as the signal to understand when the input processing has concluded, instead of returning the output right away.


<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/repl.js
patch: @@ -310,7 +310,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_);
 

filename: test/parallel/test-repl-custom-eval-previews.js
patch: @@ -0,0 +1,101 @@
+'use strict';
+
+const common = require('../common');
+const ArrayStream = require('../common/arraystream');
+const assert = require('assert');
+const { describe, it } = require('node:test');
+
+common.skipIfInspectorDisabled();
+
+const repl = require('repl');
+
+const testingReplPrompt = '_REPL_TESTING_PROMPT_>';
+
+// Processes some input in a REPL instance and returns a promise that
+// resolves to the produced output (as a string).
+function getReplRunOutput(input, replOptions) {
+  return new Promise((resolve) => {
+    const inputStream = new ArrayStream();
+    const outputStream = new ArrayStream();
+
+    const replServer = repl.start({
+      input: inputStream,
+      output: outputStream,
+      prompt: testingReplPrompt,
+      ...replOptions,
+    });
+
+    let output = '';
+
+    outputStream.write = (chunk) => {
+      output += chunk;
+      // The prompt appears after the input has been processed
+      if (output.includes(testingReplPrompt)) {
+        replServer.close();
+        resolve(output);
+      }
+    };
+
+    inputStream.emit('data', input);
+
+    inputStream.run(['']);
+  });
+}
+
+describe('with previews', () => {
+  it("doesn't show previews by default", async () => {
+    const input = "'Hello custom' + ' eval World!'";
+    const output = await getReplRunOutput(
+      input,
+      {
+        terminal: true,
+        eval: (code, _ctx, _replRes, cb) => cb(null, eval(code)),
+      },
+    );
+    const lines = getSingleCommandLines(output);
+    assert.match(lines.command, /^'Hello custom' \+ ' eval World!'/);
+    assert.match(lines.prompt, new RegExp(`${testingReplPrompt}$`));
+    assert.strictEqual(lines.result, "'Hello custom eval World!'");
+    assert.strictEqual(lines.preview, undefined);
+  });
+
+  it('does show previews if `preview` is set to `true`', async () => {
+    const input = "'Hello custom' + ' eval World!'";
+    const output = await getReplRunOutput(
+      input,
+      {
+        terminal: true,
+        eval: (code, _ctx, _replRes, cb) => cb(null, eval(code)),
+        preview: true,
+      },
+    );
+    const lines = getSingleCommandLines(output);
+    assert.match(lines.command, /^'Hello custom' \+ ' eval World!'/);
+    assert.match(lines.prompt, new RegExp(`${testingReplPrompt}$`));
+    assert.strictEqual(lines.result, "'Hello custom eval World!'");
+    assert.match(lines.preview, /'Hello custom eval World!'/);
+  });
+});
+
+function getSingleCommandLines(output) {
+  const outputLines = output.split('\n');
+
+  // The first line contains the command being run
+  const command = outputLines.shift();
+
+  // The last line contains the prompt (asking for some new input)
+  const prompt = outputLines.pop();
+
+  // The line before the last one contains the result of the command
+  const result = outputLines.pop();
+
+  // The line before that contains the preview of the command
+  const preview = outputLines.shift();
+
+  return {
+    command,
+    prompt,
+    result,
+    preview,
+  };
+}

filename: test/parallel/test-repl-custom-eval.js
patch: @@ -0,0 +1,121 @@
+'use strict';
+
+require('../common');
+const ArrayStream = require('../common/arraystream');
+const assert = require('assert');
+const { describe, it } = require('node:test');
+
+const repl = require('repl');
+
+// Processes some input in a REPL instance and returns a promise that
+// resolves to the produced output (as a string).
+function getReplRunOutput(input, replOptions) {
+  return new Promise((resolve) => {
+    const inputStream = new ArrayStream();
+    const outputStream = new ArrayStream();
+
+    const testingReplPrompt = '_REPL_TESTING_PROMPT_>';
+
+    const replServer = repl.start({
+      input: inputStream,
+      output: outputStream,
+      prompt: testingReplPrompt,
+      ...replOptions,
+    });
+
+    let output = '';
+
+    outputStream.write = (chunk) => {
+      output += chunk;
+      // The prompt appears after the input has been processed
+      if (output.includes(testingReplPrompt)) {
+        replServer.close();
+        resolve(output);
+      }
+    };
+
+    inputStream.emit('data', input);
+
+    inputStream.run(['']);
+  });
+}
+
+describe('repl with custom eval', { concurrency: true }, () => {
+  it('uses the custom eval function as expected', async () => {
+    const output = await getReplRunOutput('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', async () => {
+    const output = await getReplRunOutput('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 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_foo';
+      r.write('\n.exit\n');
+    });
+
+    assert.strictEqual(context.foo, 'global_foo');
+    delete global.foo;
+  });
+
+  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.baz = 'global_baz';
+      r.write('\n.exit\n');
+    });
+
+    assert.strictEqual(context.bar, 'global_bar');
+    assert.notStrictEqual(context.baz, 'global_baz');
+    delete global.bar;
+    delete global.baz;
+  });
+
+  /**
+   * 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');
+  });
+});

filename: test/parallel/test-repl-eval.js
patch: @@ -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);
-}