gh-88494: Reduce CLOCK_RES to 1 ms in tests by vstinner · Pull Request #118395 · python/cpython

Sadly, my change is too optimistic :-( test_asyncio failed on Windows:

FAIL: test_wait_for_handle (test.test_asyncio.test_windows_events.ProactorTests.test_wait_for_handle)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\test_asyncio\test_windows_events.py", line 190, in test_wait_for_handle
    self.assertGreaterEqual(elapsed, timeout - test_utils.CLOCK_RES)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0.4965299000000414 not greater than or equal to 0.499

loop._proactor.wait_for_handle() still has the bad 15.6 ms resolution :-(

🤖 New build scheduled with the buildbot fleet by @vstinner for commit eb4e7d3 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

Now I worry that other tests (that still use the 1ms value) will become flaky on Windows, unless you have a clear explanation why that fix is only needed in that one spot.

asyncio.BaseEventLoop.time() is time.monotonic(). On Windows, this clock now has a resolution of 100 ns, since I modified it to use QueryPerformanceCounter().

FAIL: test_wait_for_handle (test.test_asyncio.test_windows_events.ProactorTests.test_wait_for_handle)

This test calls RegisterWaitForSingleObject() which has a resolution of 15.6 ms.

I scheduled buildbot jobs to see how the change goes on more various machines.

Failure on AMD64 Windows Server 2022 NoGIL PR:

FAIL: test_wait_timeout (test.test_multiprocessing_spawn.test_misc.TestWait.test_wait_timeout)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\test\_test_multiprocessing.py", line 5145, in test_wait_timeout
    self.assertGreater(delta, timeout - CLOCK_RES)
    ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 4.996562599902973 not greater than 4.999

Maybe the problem is that the meaning of the variable CLOCK_RES is ambiguous? It looks like it gives the actual clock resolution, but it actually seems to mean the tolerance for longer (or shorter) delays. Tests depending on close-to-exact delays will always be flaky -- or if we make the tolerance too large, they will be meaningless.

While it's appealing to make the test stricter, CLOCK_RES is used to compare time.monotonic() values (resolution better than 1 us), but also to compare Windows "wait" functions (resolution of 15.6 ms). I prefer to abandon this PR. I don't want to have multiple CLOCK_RES depending on the function and/or the operating system. Let's keep the status quo of 50-100 ms tolerance.

Good call. Sorry it didn't work out, but they can't all be winners... :-)