Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bpo-47076: fix test_asyncio: test_get_cancelled() #32020

Closed
wants to merge 1 commit into from
Closed

bpo-47076: fix test_asyncio: test_get_cancelled() #32020

wants to merge 1 commit into from

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Mar 21, 2022 •
edited by bedevere-bot

@bedevere-bot bedevere-bot added the tests Tests in the Lib/test dir label Mar 21, 2022
@kumaraditya303
Copy link
Contributor Author

@sweeneyde Can you add Test with buildbots label ?

@sweeneyde sweeneyde added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 21, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sweeneyde for commit 0eb0db7 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 21, 2022
@sweeneyde sweeneyde requested a review from asvetlov March 21, 2022 06:43
@kumaraditya303
Copy link
Contributor Author

cc @vstinner

vstinner
vstinner reviewed Mar 21, 2022
View reviewed changes
Lib/test/test_asyncio/test_queues.py

async def queue_get():
started.set()
return await asyncio.wait_for(q.get(), 0.051)
Copy link
Member

Choose a reason for hiding this comment

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

is the wait_for() call still useful?

Copy link
Contributor

@asvetlov asvetlov Mar 22, 2022 •
edited

Choose a reason for hiding this comment

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

The whole test is redundant IMHO.
Please take a look at #32040 for the alternative fix that affects more problematic tests.

@asvetlov
Copy link
Contributor

Thanks for the PR!
#32040 was merged with more fixes

@asvetlov asvetlov closed this Mar 22, 2022
@kumaraditya303 kumaraditya303 deleted the fix-ci branch March 22, 2022 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner left review comments

@rhettinger rhettinger Awaiting requested review from rhettinger rhettinger is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov

Assignees
No one assigned
Labels
awaiting review tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@kumaraditya303 @bedevere-bot @asvetlov @vstinner @the-knights-who-say-ni @sweeneyde