Add unit tests for Connector class add in pull request

[tvrprasad]

#362
Add dependency on Sinon npm package for unit testing.
Fix bugs in Connector class identified by the unit tests.

[tvrprasad]

@arthurschreiber This is a pull request for unit tests for the Connector class as we discussed in - #362. Please take a look.

[arthurschreiber]

Can you convert test/unit/connector-test.coffee to javascript (ES2015)? New files should not use CoffeeScript to make the transition away from it a bit easier.

[tvrprasad]

Converted connector-test.coffee to JavaScript. eslint is passing, so I hope it's ES2015 :)

[arthurschreiber]

@tvrprasad The tests are not passing on my machine, even tho the CI runs are all green:

arthurschreiber$ node_modules/.bin/nodeunit test/setup.js test/unit/connector-test.js 

connector-test.js
✖ singleGoodIpv4NoMultiSuccess

undefined

✖ singleGoodIpv6NoMultiSuccess

undefined

✖ multipleGoodIpv4NoMultiSuccess

undefined

✖ multipleGoodIpv6NoMultiSuccess

undefined

✖ multipleGoodIpv4v6NoMultiSuccess

undefined

✖ singleGoodIpv4MultiSuccess

undefined

✖ singleGoodIpv6MultiSuccess

undefined

✖ multipleGoodIpv4MultiSuccess

undefined

✖ multipleGoodIpv6MultiSuccess

undefined

✖ multipleGoodIpv4v6MultiSuccess

undefined

✖ singleBadIpv4NoMultiFailure

undefined

✖ singleBadIpv6NoMultiFailure

undefined

✖ multipleBadIpv4NoMultiFailure

undefined

✖ multipleBadIpv6NoMultiFailure

undefined

✖ multipleBadIpv4v6NoMultiFailure

undefined

✖ singleBadIpv4MultiFailure

undefined

✖ singleBadIpv6MultiFailure

undefined

✖ multipleBadIpv4MultiFailure

undefined

✖ multipleBadIpv6MultiFailure

undefined

✖ multipleBadIpv4v6MultiFailure

undefined


FAILURES: 20/20 assertions failed (1207ms)

[arthurschreiber]

I think this is because the tests actually try to connect to the remote servers - so when running the tests on a machine that does not have network connectivity, they fail. 😞

[tvrprasad]

Correct. These tests will fail on a machine without network connectivity. They depend on being able to access some high availability sites like google.com, bing.com etc. Not ok?

I updated the tests to rely only on being able to access loopback addresses, 127.0.0.1 and ::1. So they won't need internet access anymore. You do need IPv6 loopback address enabled on your machine though.

Please checkout the updated PR. Thanks.

[tvrprasad]

@arthurschreiber
pop()
Can you make a pass and let me know if you have feedback or if I can merge this? Thanks.

[tvrprasad]

Did a rebase and resolved merge conflicts to sit on top #362.
Please take a look.

Aside. I had to do a 'git push -f' to get my changes through. Is there a better way?

[yuqinlear]

Hi, @tvrprasad @arthurschreiber any update here as one month passed? The new release is pending on this PR.

[tvrprasad]

This PR is currently waiting on @arthurschreiber to find some time to make a pass and merge or suggest changes.

[arthurschreiber]

Closing this as I added a bunch of other tests using mitm to test the new functionality.

Thank you for the work on these - I really appreciate the time spent on this, even if they didn't make it in.