Skip to content

Conversation

@dario-piotrowicz
Copy link
Member

@dario-piotrowicz dario-piotrowicz commented Dec 13, 2025

REPL magic mode has been fully removed in 2018 (#19187) there are however a couple of tests that still test magic_mode. These tests are completely redundant since they are basically simply duplicating equivalent tests of sloppy mode (since that's the mode that REPL will use when it encounters the unrecognized magic mode flag). That's why I'm removing them here.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Dec 13, 2025
Comment on lines -59 to -69

function testAutoMode() {
const { input, output } = startNewREPLServer({ replMode: repl.REPL_MODE_MAGIC, terminal: false, prompt: '> ' });

input.emit('data', 'x = 3\n');
assert.strictEqual(output.accumulator, '> 3\n> ');
output.accumulator = '';

input.emit('data', 'let y = 3\n');
assert.strictEqual(output.accumulator, 'undefined\n> ');
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is effectively a duplicate of

function testSloppyMode() {
const { input, output } = startNewREPLServer({ replMode: repl.REPL_MODE_SLOPPY, terminal: false, prompt: '> ' });
input.emit('data', 'x = 3\n');
assert.strictEqual(output.accumulator, '> 3\n> ');
output.accumulator = '';
input.emit('data', 'let y = 3\n');
assert.strictEqual(output.accumulator, 'undefined\n> ');
}

]);
}

function testMagicMode() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is effectively a duplicate of

function testSloppyMode() {
const { replServer, output } = startNewREPLServer({
prompt: testingReplPrompt,
mode: repl.REPL_MODE_SLOPPY,
});
// Cannot use `let` in sloppy mode
replServer.write(`_; // initial value undefined
var x = 10; // evaluates to undefined
_; // still undefined
y = 10; // evaluates to 10
_; // 10 from last eval
_ = 20; // explicitly set to 20
_; // 20 from user input
_ = 30; // make sure we can set it twice and no prompt
_; // 30 from user input
y = 40; // make sure eval doesn't change _
_; // remains 30 from user input
`);
assertOutput(output, [
'undefined',
'undefined',
'undefined',
'10',
'10',
'Expression assignment to _ now disabled.',
'20',
'20',
'30',
'30',
'40',
'30',
]);
}

@dario-piotrowicz dario-piotrowicz changed the title test: remove unneccessary reply magic_mode tests test: remove unneccessary repl magic_mode tests Dec 13, 2025
@dario-piotrowicz dario-piotrowicz force-pushed the dario/repl-magic-mode-tests branch from 8b14c72 to b262504 Compare December 13, 2025 19:39
@codecov
Copy link

codecov bot commented Dec 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (4f24aff) to head (b262504).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #61053   +/-   ##
=======================================
  Coverage   88.53%   88.53%           
=======================================
  Files         703      703           
  Lines      208546   208546           
  Branches    40217    40222    +5     
=======================================
  Hits       184634   184634           
+ Misses      15926    15913   -13     
- Partials     7986     7999   +13     

see 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@BridgeAR BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Dec 16, 2025
@dario-piotrowicz dario-piotrowicz removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 16, 2025
@dario-piotrowicz dario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 16, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025
@nodejs-github-bot
Copy link
Collaborator

@dario-piotrowicz dario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025
@nodejs-github-bot
Copy link
Collaborator

@dario-piotrowicz dario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025
@nodejs-github-bot
Copy link
Collaborator

@aduh95
Copy link
Contributor

aduh95 commented Dec 20, 2025

@dario-piotrowicz please do not repeatedly add the request-ci Add this label to start a Jenkins CI on a PR. label, that restarts all the Jankins jobs, risking oversubscribing our Jenkins infra and increasing the chance of landing a change introducing more flakiness.
Instead, you can add the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label and wait for someone to check the CI result and chose to resume only if the failure is indeed a fluke.

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 20, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 20, 2025
@nodejs-github-bot nodejs-github-bot merged commit e5b6f89 into nodejs:main Dec 20, 2025
82 of 85 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in e5b6f89

@dario-piotrowicz dario-piotrowicz deleted the dario/repl-magic-mode-tests branch December 20, 2025 16:21
MatricalDefunkt pushed a commit to MatricalDefunkt/node that referenced this pull request Dec 22, 2025
PR-URL: nodejs#61053
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants