add unit test for file #6
Conversation
|
Hey @phapdinh, thanks for the contribution! While I think adding tests to this library is a good thing, I am not sure if this is the correct way to test this. First I think that what you want to test is that the prototype chain was modified, and this does not test that. Secondly, the behavior we really want to test is that it works in all the supported environments, since that is the only reason to use this module. If you want to move this forward I would look at structuring the tests such that they test these things. I would look at either using docker to test older node versions or a setup with travis for the same. Also a small nitpick: preferably it would follow the mocha standard of putting the tests in a |
| @@ -5,7 +5,7 @@ | |||
| "main": "index.js", | |||
| "typings": "index.d.ts", | |||
| "scripts": { | |||
| "test": "echo \"Error: no test specified\" && exit 1" | |||
| "test": "mocha *.test.js" | |||
There was a problem hiding this comment.
If you name the file test.js instead of index.test.js you don't have to explicitly state the name here, and it will be aligned more closely with how the tests files are usually named by following the default settings.
edit: just saw that wesleytodd had already mentioned this
|
It would be nice to get some tests in, even if it just tests Node.js, so that we can get this package integrated into the CITGM testing: nodejs/citgm#631 I'd be happy to help out with anything that needs to be done 👍 We could also look at adding tests for the browser using something like Browserling? Still, I think it would be a step in the right direction to get a Node.js-test in |
|
Closed in 6d8bbc4 |
This creates a unit test for the file using mocha