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

tests: use rm -rf instead of rmdir in teardown #2010

Merged
merged 1 commit into from Feb 27, 2019
Merged

tests: use rm -rf instead of rmdir in teardown #2010

merged 1 commit into from Feb 27, 2019

Conversation

jcsahnwaldt
Copy link
Contributor

Most teardown scripts use rm -rf to delete directories the tests created. test/fast/Listing versions/teardown and test/fast/Listing paths/teardown used rmdir instead, which fails in most cases because the directories are not empty.

This PR replaces rmdir by rm -rf in these scripts. It also drops the >/dev/null 2>&1 part, because rm -f never prints any errors anyway.

Additionally, I removed the line unalias nvm_has_system_node from test/fast/Listing versions/teardown because the tests don't create an alias (instead, they redefine the function nvm_has_system_node), and it looks like the new definition of nvm_has_system_node does not exist anymore after the tests have run (probably because the each test runs in its own process). But I only tested this on my machine by running the tests as scripts. For example:

$ cd ~/.nvm/test/fast/'Listing versions'
$ ./'Running "nvm ls" should include "system" when appropriate'
$ type nvm_has_system_node
nvm_has_system_node is a function
nvm_has_system_node () 
{ 
    [ "$(nvm deactivate >/dev/null 2>&1 && command -v node)" != '' ]
}

In case I missed something and the behavior is different in other shells or in urchin, I'd be happy to add the unalias line again.

 - fix test 'Running "nvm ls" should display all installed versions.': only expect versions created by this test, but no versions created by other tests
@jcsahnwaldt
Copy link
Contributor Author

The fix in teardown uncovered a bug in the test Running "nvm ls" should display all installed versions.: Each test should be self-contained and only check the versions it created, but because teardown didn't clean up properly after each test, this test was made to expect versions created by other tests. The latest commit fixes that.

@ljharb ljharb added the testing Stuff related to testing nvm itself. label Feb 27, 2019
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks!

@ljharb ljharb merged commit a1abfd1 into nvm-sh:master Feb 27, 2019
@jcsahnwaldt jcsahnwaldt deleted the teardown-rm-fix branch February 27, 2019 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Stuff related to testing nvm itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants