nvm-sh / nvm

Node Version Manager - POSIX-compliant bash script to manage multiple active node.js versions
MIT License
79.26k stars 7.93k forks source link

Testing the install script #552

Open xcambar opened 9 years ago

xcambar commented 9 years ago

I was thinking about @ljharb's comment about the install script being untested, and had a look at how we could test it.

The testing tools that seem of interest to me are Roundup, shunt and the classic shunit2. Feedback about this list is more than welcome, I don't have much experience in unit-testing shell scripts.

The strategy is currently as follows:

curl_mock_success() {
  exit 0
}
curl_mock_failure() {
  exit 1
}

alias curl=curl_mock_success #or curl_mock_failure, depending on the test

This is certainly not perfect and there is probably more to do than the described above, but I'm willing to give it a try and wanted to have your opinion on this firsthand.

xcambar commented 9 years ago

There are trouble with mocking. I tested it too fast. I'll come back with a more robust solution.

xcambar commented 9 years ago

OK, I was just a setting away.

The following Gist provides an example about how we can use mocking in the tests: https://gist.github.com/xcambar/a119a8ed985e14521f51

I'll keep trying to have this work until further notice from you guys.

ljharb commented 9 years ago

curl would need to be mocked with the actual content of nvm.sh, for example - this might be a bit trickier than you think.

That said, it should certainly be explored, and this issue is a great place to do it.

xcambar commented 9 years ago

You can follow the progress on this branch: https://github.com/xcambar/nvm/tree/install_tests

Here's a brief summary:

The branch above is still a WIP but I'll gladly take feedback as I'm working on it.

Will PR when coverage is complete

ljharb commented 9 years ago

I'd be very concerned about changing the script, and adding/changing tests, in the same branch. Preferably, we could get as much of the tests out without any refactoring as possible - or, get a number of small PRs of changes (that could be confirmed to not break) out separately. I strongly prefer many small PRs to any large ones.

xcambar commented 9 years ago

We can do that, but it'll be more psychological than anything. We don't currently have any non-regression validation and we can't be confirmed that nothing breaks because there are no tests.

Adding integration tests for the install script (in its current state) to the existing test suite, crossing the various options, methods and shells seems to be the right way to address your concerns.

After all, if it's not unit tested, it can be battle tested :)