Use local broker instead of test.mosquitto.org for tests

[empicano]

There seems to be a race condition in the test_client_unsubscribe test when it runs in the test matrix. This surfaced with the switch to poetry, maybe I messed up something during the switch as well.

When I run the workflows one after the other everything is fine, but when they run at the same time they often fail.

Maybe this could be mitigated by spinning up our own mosquitto broker via Docker for each test run instead of using the public "test.mosquitto.org" one.

[frederikaalund]

It does sound "race condition"-like since it only happens sometimes.

Maybe this could be mitigated by spinning up our own mosquitto broker via Docker for each test run instead of using the public "test.mosquitto.org" one.

That's a great idea. 👍 Would make the tests more resilient to third party disturbances (e.g., if test.mosquitto.org is overloaded).

[JonathanPlasse]

I do not know if we can use Docker when testing on Windows.

[frederikaalund]

I just got bit by this! 😄

I think I found the culprit though: The tests all ran as python 3.10. In turn, that caused the TOPIC_HEADER to contain 3.10 for all our tests. In turn, our test suite published to the same topic in parallel. That topic-based race condition triggered in test_client_unsubscribe.

I fixed it by adding these lines to the test workflow:

      - name: Make Poetry use the active python version 
        run: poetry config virtualenvs.prefer-active-python true

It makes poetry use the python version that we just installed. With this in place, I can see that the tests actually execute with the respective python versions (as they should). This fixes the race condition since TOPIC_HEADER is unique again.

---

I still think it's a good idea to use something else than test.mosquitto.org for this. :) Let's keep this issue open for that.

[empicano]

That's good detective work 😎 I totally missed that!

The change to the setup script broke the docs workflow, hehe. I tried around with the workflows with your insight in mind and noticed that the problem stems from the order that the setup-python and install-poetry actions are run.

Initially, install-poetry ran first, as proposed in the setup-python docs about caching packages. Now, setup-python runs first and poetry directly uses the correct Python version without needing to change the config! We lose the caching that's built into the setup-python action, but I recreated that using the cache action.

[frederikaalund]

Initially, install-poetry ran first, as proposed in the setup-python docs about caching packages. Now, setup-python runs first and poetry directly uses the correct Python version without needing to change the config! We lose the caching that's built into the setup-python action, but I recreated that using the cache action.

That makes sense! That's the proper fix. I never understood why it "just didn't work" since it was more or less copy-paste from the example code of snok/install-poetry. Nice find and well done. 👍

[empicano]

I do not know if we can use Docker when testing on Windows.

I tried it out (#248) and you're right, it doesn't work in GitHub Actions. MacOS has problems, too.

I found the official GitHub Actions documentation that states that Docker only works on Linux runners. The problem seems to be that GitHub Action runners use Windows Server and MacOS Server which have restrictions on running Docker on them.

That's sad, because you can make the tests very elegant on Linux otherwise, and they should work locally on Windows and MacOS as well (I only tested on MacOS). Other MQTT libraries either only test on Linux in the CI or use a hosted broker like we do; paho-mqtt implements a fake broker as far as I understood.

What do you think, having the CI run the tests on Windows and MacOS is probably more important than not using the network, or not?

[JonathanPlasse]

What do you think, having the CI run the tests on Windows and MacOS is probably more important than not using the network, or not?

That was what I thought when I added the tests with the mosquito broker.

Another solution would be to install a broker implemented in Python like amqtt, no Docker is needed.
And the real solution would be having our own broker.

[empicano]

Makes sense 👍 I think for now it's ok to leave it as is then. Let's keep this issue open in case something changes on the Docker topic