Closed karlbright closed 4 years ago
Sorry for late response, I'll find sometimes this weekend to look at this PR. And thank you so much! This is definitely needed!
@huytd No worries. Fixed merge conflicts so it should be all good to go.
@karlbright Thanks for bringing this up. Regarding to the test framework, honestly I never use or heard about tape before. But looking at it, I think I like the lightweight-ness of it. So I'll give it a try in this project then.
@huytd This has been fixed up to use strings for IDs. Updating the tests to use strings caught an issue with the implementation of the move command, where ID's were being returned with their whitespace due to the matching group setup. I've fixed that up in 4fa0e5c to ensure the tests pass correctly as a result of this PR.
Awesome. Thank you so much. Merging now.
Adds testing around the commands helper, using the commands listed within the help text.
I wanted to look at expanding on functionality around some of these commands, but given the lack of tests I thought it best to start there and make it easy to refactor or expand on in a future pull requests.
I'm not sure on your thoughts for testing framework, but
tape
has always served me well and is easy enough to follow. Given the state of the codebase currently, it seemed a good fit.