Pass options to ascoltatores #55
Conversation
Extra parameter is optional. memory and mqtt ascoltatores updated. Test for memory ascoltatores, mqtt test needs to be done after mosca is updated to take advantage of the options (e.g. to pass the qos through).
|
|
||
| Object.defineProperty(this, "publish", { | ||
| get: function() { return this._publish; }, | ||
| set: this._set_publish |
There was a problem hiding this comment.
Why are you adding a public set for the publish function?
Is that needed somewhere?
There was a problem hiding this comment.
Test fails without it (spy fails to set its publish intercept):
- ascoltatori should delegate to the use ascoltatore for 'pub':
TypeError: Cannot set property publish of # which has only a getter
at Object.wrapMethod (/home/david/Private/hub/node_modules/mosca/node_modules/ascoltatori/node_modules/sinon/lib/sinon.js:80:30)
at Object.spy (/home/david/Private/hub/node_modules/mosca/node_modules/ascoltatori/node_modules/sinon/lib/sinon/spy.js:42:22)
at Object.spy (/home/david/Private/hub/node_modules/mosca/node_modules/ascoltatori/node_modules/sinon/lib/sinon/collection.js:88:39)
at Context. (/home/david/Private/hub/node_modules/mosca/node_modules/ascoltatori/test/ascoltatori_spec.js:26:28)
at Test.Runnable.run (/usr/lib/node_modules/mocha/lib/runnable.js:213:32)
at Runner.runTest (/usr/lib/node_modules/mocha/lib/runner.js:351:10)
at Runner.runTests.next (/usr/lib/node_modules/mocha/lib/runner.js:397:12)
at next (/usr/lib/node_modules/mocha/lib/runner.js:277:14)
at Runner.hooks (/usr/lib/node_modules/mocha/lib/runner.js:286:7)
at next (/usr/lib/node_modules/mocha/lib/runner.js:234:23)
at Runner.hook (/usr/lib/node_modules/mocha/lib/runner.js:249:7)
at Hook.Runnable.run (/usr/lib/node_modules/mocha/lib/runnable.js:215:5)
at next (/usr/lib/node_modules/mocha/lib/runner.js:243:10)
at Runner.hook (/usr/lib/node_modules/mocha/lib/runner.js:254:5)
at process.startup.processNextTick.process._tickCallback (node.js:244:9)
There was a problem hiding this comment.
I see, but having a real ascoltatore is not really needed in that test.
Could you please fix the ascoltatori_spec.js not to override that, and maybe use a fake one all the way?
Is it the only problem?
There was a problem hiding this comment.
Depends whether you want people to be able to set the publish method in the constructor (this.publish = ...) as an alternative to using the prototype.
There was a problem hiding this comment.
Actually not, the supported way is using the prototype.
Do you see any actual use case for this?
|
I removed the getters, only using the prototype now and added fake ascoltatore for the test |
It was causing a double 'ready' event emission. Instead, make the functions to set up publish and subscribe methods available to deriving classes and call them in DecoratorAscoltatore.
|
Fixed double ready emits by reverting to not inheriting from DecoratorAscoltatore from AbstractAscoltatore. |
Pass options to each Ascoltatore#publish and #subscribe
Extra parameter is optional.
memory and mqtt ascoltatores updated.
Test for memory ascoltatores, mqtt test needs to be done after mosca is updated to take advantage of the options (e.g. to pass the qos through).
Issue #53