Improve test reliability and fix multi-threading issues #1150
Conversation
This makes the unit tests more reliable by allowing events to be received in different orders and by allowing additional (non-expected) events.
This improves the performance of the winapi observer by using a second thread and by using double-buffering. The time window for events to be lost due to processing outside the ReadDirectoryChangesW() call should be smaller. The events buffer is reused rather than creating a new one on every call.
Use a lock and event to ensure that BaseThread.start() and BaseThread.stop() work reliability when called in different orders or concurrently from different threads.
|
@gorakhargosh we know this is a huge diff, but I think it's going to be worthwhile to go through the work of merging this. In particular, @nascheme thinks that this PR removes the need to use Would it be helpful to have a PEP-like document to read over? Something else? We'd like to make this as easy as possible for you to review the PR. Also - @nascheme: this fixes #1128, right? Also to reduce confusion, I'm going to go ahead and close my old open PR that attempts to fix the tests in favor of this one. |
|
FYI Gorak is out of business since at least 2018, I'm the maintainer since then. This is a huge diff, indeed. And splitting the work in smaller PR is welcome (as it is already done). I'll review them. If anyone wants to review too, be my guest: more eyes are always better :) |
|
Sorry for the extraneous ping! |
|
#1131 merged 👍 |
3 participants
Summary of changes:
events_checker()test fixture, use in unit testsWith this PR applied, the tests are running reliably for me on Linux, MacOS and Windows. I believe the flaky test plugin could be removed. I've not run multi-threaded tests yet but this should give us a more solid base to work from.
I can split these into separate PRs if that's easier to review or to merge. I've already cleaned up the commits to be separate logical changes. E.g. each commit could be a PR. I've discussed some of the changes with @ngoldbaum and he approved me building on his PRs.
I split the two big changes into their own PRs, see them for more details on those changes.
Each of those can be merged separately. I'd recommend that GH-1131 is merged first. It's a pretty simple change (looks large due to the addition of the
pythoncapi_compat.hheader but actual lines of code changed is small and pretty easy to review).