Skip to content

Conversation

@kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Sep 8, 2023

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Let's kill more short sleeps! They always cause problems.

Copy link
Contributor

@itamaro itamaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
Can you explain why this was flakily failing on Windows, on why this fixes it? It's not obviously apparent to me from the code change

@kumaraditya303
Copy link
Contributor Author

Can you explain why this was flakily failing on Windows, on why this fixes it? It's not obviously apparent to me from the code change

On Windows the monotonic clock is low precision so it distorts the test.

@kumaraditya303 kumaraditya303 enabled auto-merge (squash) September 8, 2023 15:38
@vstinner
Copy link
Member

vstinner commented Sep 8, 2023

I confirm that the fix works as expected.

On Linux, it's hard for me to reproduce the issue. But on Windows, it's quite easy with the command:

python -m test test_asyncgen -m test_async_gen_asyncio_gc_aclose_09 -j12 --forever 

It fails in a few seconds. I ran the test in a Windows VM with 2 CPUs (running on my laptop which has 12 threads / 6 CPU cores).

With this PR, the test no longer fails. I interrupted the test for a few minutes:

...
0:06:21 load avg: 20.99 [807] test_asyncgen passed
0:06:22 load avg: 20.99 [808] test_asyncgen passed
0:06:23 load avg: 21.02 [809] test_asyncgen passed

To confirm the fix, I ran a second test on the PR on the whole test_asyncgen test module (without filtering on -m test_async_gen_asyncio_gc_aclose_09). I interrupted the test since it worked! test_asyncgen now looks rock solid.

...
0:05:39 load avg: 22.00 [492] test_asyncgen passed
0:05:39 load avg: 22.00 [493] test_asyncgen passed
0:05:40 load avg: 21.95 [494] test_asyncgen passed
0:05:41 load avg: 21.86 [495] test_asyncgen passed
0:05:41 load avg: 21.86 [496] test_asyncgen passed

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kumaraditya303 kumaraditya303 merged commit ccd4862 into python:main Sep 8, 2023
@miss-islington
Copy link
Contributor

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 8, 2023
…se_09` test (pythonGH-109142)

Use `asyncio.sleep(0)` instead of short sleeps.
(cherry picked from commit ccd4862)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 8, 2023
…se_09` test (pythonGH-109142)

Use `asyncio.sleep(0)` instead of short sleeps.
(cherry picked from commit ccd4862)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@bedevere-bot
Copy link

GH-109149 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Sep 8, 2023
@bedevere-bot
Copy link

GH-109150 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Sep 8, 2023
@kumaraditya303 kumaraditya303 deleted the async branch September 8, 2023 16:30
vstinner pushed a commit that referenced this pull request Sep 8, 2023
…ose_09` test (GH-109142) (#109150)

GH-109067: fix randomly failing `test_async_gen_asyncio_gc_aclose_09` test (GH-109142)

Use `asyncio.sleep(0)` instead of short sleeps.
(cherry picked from commit ccd4862)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Yhg1s pushed a commit that referenced this pull request Sep 12, 2023
…ose_09` test (GH-109142) (#109149)

GH-109067: fix randomly failing `test_async_gen_asyncio_gc_aclose_09` test (GH-109142)

Use `asyncio.sleep(0)` instead of short sleeps.
(cherry picked from commit ccd4862)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir topic-asyncio

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants