Proxy to masterPod instance if no DNS mapping#96
Conversation
…ute-to-master-with-no-dns-mapping
* origin/master:
3.1.3
Fixed bad bug where the replace of {{DOMAIN}} was not being ran because the script wasn't replacing it's contents properly.
Update log msg fields to reflect new module method org
Change test to complete when function calls back
Conflicts:
lib/models/api.js
There was a problem hiding this comment.
this function doesn't need to exist.
There was a problem hiding this comment.
That method is invoked by the code under test and for the test to be successful it needs to return undefined. What do you mean by function doesn't need to exist -- the function doesn't need to be stubbed? Not stubbed in this way?
There was a problem hiding this comment.
you could do sinon.stub(mongo.constructor, 'findAssociationShortHashByElasticUrl') and that'd be the same of what you just did.
There was a problem hiding this comment.
well to be clear
sinon.stub(mongo.constructor, 'findAssociationShortHashByElasticUrl').returns(undefined)
There was a problem hiding this comment.
is .returns(undefined) needed? Isn't that implicit? I'm not against being explicit here, but we should get some standard for how we approach that case so we are consistent.
|
reviewed: have minor nits but code looks good. will check the box in the description when satisfied |
There was a problem hiding this comment.
can be done with sinon.stub(mongo.constructor, 'findMasterPodBranch).returns(mockNaviEntry)
|
so the main reason I would like the test changes is not because the way you are doing it is wrong (it does meet the requirements and works) however there are patterns which most of the other repos follow. they are not documented which is why its hard to follow (ill try to write something quick up) but we should try and keep our testing patterns consistent across all repos so anyone can easily jump in and follow some standard runnable conventions to write test, unit/funtional/integration test |
|
@anandkumarpatel I agree with the patterns you've explained so I tried to move a substantial number of the stubs & assertions over to using the preferred sinon APIs. I'll make use of those discussed patterns and sinon APIs in all future testing. |
There was a problem hiding this comment.
No reason to do the if here. Since the beforeEach will always stub it.
There was a problem hiding this comment.
This is here because some of the methods under test assumed it wasn't stubbed... so some of the tests might have restored it themselves before the afterEach. Not ideal, I know.
3 participants
reviewed