Repository navigation
Conversation
Signed-off-by: Maxime Tolos <104535850+maxitoon@users.noreply.github.com>
nfischer
requested changes
Oct 14, 2026
| t.is(shell.env.SHELLJS_TEST, process.env.SHELLJS_TEST); | ||
| }); | ||
|
|
||
| [undefined, '', 'previous directory'].forEach(oldpwd => { |
Member
There was a problem hiding this comment.
Please unroll this loop into separate test cases. If you want to reuse code, then you can declare a helper function.
| const result = shell.cd('-'); // Haven't changed yet, so there is no previous directory | ||
| const oldpwd = process.env.OLDPWD; | ||
| delete process.env.OLDPWD; | ||
| t.teardown(() => { |
Member
There was a problem hiding this comment.
I'm not sure this teardown step is really necessary
Author
|
Updated in commit 126b239. test/env.js now has separate explicit absent, empty, and nonempty inherited OLDPWD cases. test/cd.js runs the missing-OLDPWD case in an isolated child, removing the need for teardown. Local Linux/Node 20 verification: focused tests 15 passed; full suite 644 passed, 6 skipped; lint passed. Hosted CI for this new commit is pending verification. Correction: my earlier comment was posted before the commit was pushed; the update is now present on this PR. |
Signed-off-by: Maxime Tolos <104535850+maxitoon@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1030.
Importing ShellJS currently deletes
process.env.OLDPWD, even when it was supplied by the caller. This changes the importing application's environment and can break consumers such as webpack's EnvironmentPlugin.Remove that import-time deletion. Existing
cd()behavior remains responsible for updatingOLDPWDafter a successful directory change. Consequently,cd('-')can also use a previous directory inherited from the calling environment rather than treating it as absent.Regression tests run imports in fresh child processes, covering absent, empty, and nonempty values, plus
cd('-')with an inherited directory. The existing missing-previous-directory test now establishes and restores its own environment explicitly.Validation on Linux / Node 20.19.2:
npm test— 640 passing, 6 skipped; lint passes.node node_modules/ava/entrypoints/cli.mjs test/env.js test/cd.js— 15 passing.npm test— 644 passing, 6 skipped; lint passes.npm run check-node-supportandgit diff --check— pass.Limitations: other Node versions and native Windows/macOS have not been run locally. The node-support check is static, not a runtime matrix. npm 12.0.2 warned that this Node version is unsupported by its CLI, although the listed commands completed successfully. These are local results, not a claim of hosted CI success.
Implementation, tests, and independent local reviews used AI assistance.