Skip to content

GH-109067: fix randomly failing test_async_gen_asyncio_gc_aclose_09 test #109142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 8, 2023

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 <[email protected]>
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 <[email protected]>
@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 <[email protected]>
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 <[email protected]>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants