moscajs / aedes

Barebone MQTT broker that can run on any stream server, the node way
MIT License
1.75k stars 228 forks source link

[question] does the test "unsubscribe throws error" valid? #940

Open gnought opened 4 months ago

gnought commented 4 months ago

https://github.com/moscajs/aedes/blob/bba422b7733d9946566647c7300c1b293e8cd8cf/test/client-pub-sub.js#L664-L688

The mocked broker.unsubscribe is only called by broker.close.

  1. Does it expect to be called by client.subscribe ?
  2. The Error does not be raised in client error event. Bug?
  3. The callback in client.subscribe does not contain any error argument, does the message in t.pass make sense?
robertsLando commented 4 months ago

By reading it makes no sense to me too. I think it should be something like:

client.unsubscribe({ 
         topic: 'hello', 
         qos: 0 
       }, function (err) { 
         t.equal(err.message, 'error', 'throws error') 
       }) 
gnought commented 4 months ago

@robertsLando so do I, however the callback call doesn't have err parameter.

robertsLando commented 4 months ago

I should try it so, I dunno sincerly... let me know if you fix it