GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix potential flakiness in test_run_until_complete_baseexception by fantix · Pull Request #100148 · python/cpython

Conversation

@fantix

@fantix fantix commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

Refs #24477, use a more reliable way to run the test.

If the covering issue happens again, the loop would be requested to stop in the first iteration and got stopped after the first iteration. In that case, our func() in the second iteration won't be called.

  • skip issue
  • skip news

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@fantix
Fix potential flakiness in test_run_until_complete_baseexception
6e3ae5c
@fantix
fantix requested review from 1st1, asvetlov, gvanrossum and kumaraditya303 as code owners December 9, 2022 21:57
@bedevere-bot bedevere-bot added awaiting review tests Tests in the Lib/test dir labels Dec 9, 2022
@python python deleted a comment from netlify Bot Dec 10, 2022
@gvanrossum

gvanrossum commented Dec 10, 2022

Copy link
Copy Markdown
Member

@kumaraditya303 Can you review this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303 kumaraditya303 added skip issue skip news labels Dec 10, 2022
@kumaraditya303 kumaraditya303 self-assigned this Dec 10, 2022
kumaraditya303
kumaraditya303 approved these changes Dec 10, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Dec 10, 2022
@kumaraditya303
kumaraditya303 merged commit a9bad4d into python:main Dec 10, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Dec 10, 2022
@kumaraditya303

kumaraditya303 commented Dec 10, 2022

Copy link
Copy Markdown
Contributor

Can you review this?

Done & merged.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303

kumaraditya303 commented Dec 10, 2022

Copy link
Copy Markdown
Contributor

This is indeed better than sleeping for a small delay.

fantix reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 pushed a commit to kumaraditya303/cpython that referenced this pull request Dec 10, 2022
@fantix
Fix potential flakiness in test_run_until_complete_baseexception (p…
50b08d5
…ython#100148)
kumaraditya303 pushed a commit to kumaraditya303/cpython that referenced this pull request Dec 10, 2022
@fantix
Fix potential flakiness in test_run_until_complete_baseexception (p…
f28e537
…ython#100148)
@fantix
fantix deleted the asyncio-close-test-fix branch December 10, 2022 15:32
@gvanrossum

gvanrossum commented Dec 10, 2022

Copy link
Copy Markdown
Member

Yup, a very insightful solution. Thanks @fantix !

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Dec 10, 2022

Copy link
Copy Markdown
Member

This PR appears to have resulted in an empty commit, likely because the PR branch was not fully up to date with main: a9bad4d :/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Dec 10, 2022

Copy link
Copy Markdown
Member

This PR appears to have resulted in an empty commit, likely because the PR branch was not fully up to date with main: a9bad4d :/

Very weird. Especially since when I check out main and pull to head, the change is there, and attributed to this PR. So maybe the problem is with the GitHub UI? I saw some mention of enpty branches on Discord but chose to ignore it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Dec 10, 2022

Copy link
Copy Markdown
Member

Oh, weird. Yes, I see the same. It must be something wrong with the GitHub UI. My sincere apologies for the false accusation, @fantix!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@fantix

fantix commented Dec 10, 2022

Copy link
Copy Markdown
Contributor Author

No worries @AlexWaygood , thank you for being careful ❤️

AlexWaygood reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Dec 12, 2022

Copy link
Copy Markdown
Member

Hm, there's definitely something weird though. I see three commits for this PR in the main branch:

commit a9bad4d28413666edc57551dd439bca6a6a59dd9
Author: Fantix King <[email protected]>
Date:   Sat Dec 10 02:05:24 2022

    Fix potential flakiness in `test_run_until_complete_baseexception` (#100148)
    
    

commit f28e537125941d9f8894ec01b5e87252e824a688
Author: Fantix King <[email protected]>
Date:   Sat Dec 10 02:04:50 2022

    Fix potential flakiness in `test_run_until_complete_baseexception` (#100148)
    
    

commit 50b08d5b5fd3e280b99b97a42d07089a64bf12c6
Author: Fantix King <[email protected]>
Date:   Sat Dec 10 02:04:22 2022

    Fix potential flakiness in `test_run_until_complete_baseexception` (#100148)
    
    

diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py
index 532a7e5385..3b4026cb73 100644
--- a/Lib/test/test_asyncio/test_base_events.py
+++ b/Lib/test/test_asyncio/test_base_events.py
@@ -868,7 +868,7 @@ def func():
             self.loop.stop()
             func.called = True
         func.called = False
-        self.loop.call_later(0.01, func)
+        self.loop.call_soon(self.loop.call_soon, func)
         self.loop.run_forever()
         self.assertTrue(func.called)

Two of those are empty. What kind of GitHub hiccup could have caused that???

fantix and AlexWaygood reacted with confused emoji
All reactions
  • 😕 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303

kumaraditya303 commented Dec 13, 2022

Copy link
Copy Markdown
Contributor

Possibly we can report this to GitHub somehow or ask anyone from GitHub to look at this? I don't know how to do it though.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rwgk pushed a commit to rwgk/cpython that referenced this pull request Mar 11, 2023
@fantix @rwgk
Fix potential flakiness in test_run_until_complete_baseexception (p…
2978893
…ython#100148)
rwgk pushed a commit to rwgk/cpython that referenced this pull request Mar 11, 2023
@fantix @rwgk
Fix potential flakiness in test_run_until_complete_baseexception (p…
fc2eb26
…ython#100148)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@kumaraditya303 kumaraditya303

Labels

skip issue skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@fantix @gvanrossum @kumaraditya303 @AlexWaygood @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.