search

eclipse-paho / paho.mqtt.golang

Other
2.77k stars 534 forks source link

Add SimulateConnectionLoss interface to help with testing registered callbacks #568

Closed gangabhavanik closed 2 years ago

gangabhavanik commented 2 years ago

This interface simply calls existing internal interface internalConnLoss.

Testing: I have tested it by running mosquitto mqtt broker and running 'go test'

$ go test 
...
connection lost before Publish completed
choke is waiting
choke is waiting
i2: 2
PASS
ok      github.com/eclipse/paho.mqtt.golang     25.786s

I am adding this interface to give me the ability to unit test OnConnectHandler callback code in the project that I am currently working on.

gangabhavanik commented 2 years ago

Let me close this and submit with id that I have registered on eclipse.

MattBrittan commented 2 years ago

Before you resubmit this please consider what benefits this delivers and state them in the pull description (or, ideally, open an issue first stating the issue you are trying to address so we can discuss). Changing this to SimulateConnectionLoss adds a new externally accessible function (internalConnLost is private).

internalConnLost was implemented for testing only (hence the internal in the name). As a result the function itself has not been thoroughly tested and using it may well lead to deadlocks etc. Due to the way this library is written (heaps of options) it is difficult to predict how its used and very easy to introduce additional deadlocks when exposing new functionality. As such it's important that there is a clear rationale for adding functionality and it needs to be well tested.

gangabhavanik commented 2 years ago

Thank you. I'll open an issue stating what I am trying to solve.