Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nvm unload fast test cleanup #2332

Closed
wants to merge 0 commits into from
Closed

Fix nvm unload fast test cleanup #2332

wants to merge 0 commits into from

Conversation

reasonablytall
Copy link
Contributor

Noticed that ./test/fast/Running "nvm unload" should unset all function and variables. would leave behind ./before.tmp and ./after.tmp files and clutter the git status. This fixes the cleanup behavior.

Let me know if it'd be preferable to move the temporary files into a test/**/test_output folder which is covered by .gitignore, or something like that.

@@ -5,7 +5,7 @@ set -ex
BEFORE="./before.tmp"
AFTER="./after.tmp"

cleanup () { echo rm -f "${BEFORE}" "${AFTER}"; }
cleanup () { rm -f "${BEFORE}" "${AFTER}"; }
Copy link
Member

@ljharb ljharb Oct 22, 2020

Choose a reason for hiding this comment

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

ha, no this was just me debugging locally and forgetting to remove my echo. good catch.

@reasonablytall

This comment has been minimized.

@ljharb ljharb closed this Oct 23, 2020
@ljharb
Copy link
Member

ljharb commented Oct 23, 2020

oops!

@ljharb
Copy link
Member

ljharb commented Oct 23, 2020

As soon as tests are passing on master, I'll land this directly.

@ljharb ljharb added hacktoberfest-accepted If you're interested in a free shirt, this PR counts towards it. testing Stuff related to testing nvm itself. labels Oct 23, 2020
@ljharb
Copy link
Member

ljharb commented Oct 23, 2020

Landed as f2c5ce4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted If you're interested in a free shirt, this PR counts towards it. testing Stuff related to testing nvm itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants