Fix issue with local options being overwritten with Nexmo class #311
Conversation
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
=======================================
Coverage 90.44% 90.44%
=======================================
Files 25 25
Lines 827 827
=======================================
Hits 748 748
Misses 79 79
Continue to review full report at Codecov.
|
| appendToUserAgent: "EXT", | ||
| debug: true | ||
| }; | ||
| new Nexmo( | ||
|
|
||
| let passedOptions = Object.assign({}, localOptions); |
There was a problem hiding this comment.
Why do we Object.assign here? We should pass localOptions to the constructor then assert that they were not changed
There was a problem hiding this comment.
Because localOptions === localOptions regardless, and the test will always pass. I need to compare it to the original without the original changing.
There was a problem hiding this comment.
Can you help me understand, also? From what I am reading this test tests the validity of Object.assign() but it's not testing it from within the constructor of the Nexmo object. Wouldn't we want our test to test this function call from within the constructor, specifically?
There was a problem hiding this comment.
Ah, I see that you are actually testing both in this block. Okay, as long as the nexmo object's values are also tested, I'm good with it.
There was a problem hiding this comment.
@bencgreenberg It's an unfortunate side effect of javascript - you have to store the original as a separate object to validate if the two objects remain the same.
Fixes #295