test: fix flaky sequencer unit tests

[plamut]

Fixes #178.

Patching the client under test should be done on an instance used in a test, not on the instance's class - patching the latter can cause all other instances of the same class to share the patched method, possibly interfering with the patched method's call count.

This can probably be shipped along the new major version, as it does not interfere with the changes made there.

PR checklist

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

[pradn]

Ah! This makes sense. I assumed the patch would be reset between tests, but I guess there's no reason why that should be. Thanks for the fix!

[plamut]

I assumed the patch would be reset between tests, but I guess there's no reason why that should be.

Actually, the patch is reset between the tests (that's its purpose after all), but the problem is that patching a method on a class affects all of its instances.

The issue here was that some of the other tests create their own Thread-PubSubBatchCommitter threads that invoke the _commit_sequencers() method, but if that method is patched on the class, that mocked method is shared between tests. That occasionally messed up the call count when some of those Thread-PubSubBatchCommitter threads were still running.

[pradn]

I see: patching a class is modifying "global state", which leads to conflicts when multiple threads are using that class-level patched method. Thank you for the explanation!