Skip to content

gh-95736: Fix event loop creation in IsolatedAsyncioTestCase#96033

Merged
miss-islington merged 2 commits intopython:mainfrom
serhiy-storchaka:unittest-async-case-event-loop-setup
Aug 17, 2022
Merged

gh-95736: Fix event loop creation in IsolatedAsyncioTestCase#96033
miss-islington merged 2 commits intopython:mainfrom
serhiy-storchaka:unittest-async-case-event-loop-setup

Conversation

@serhiy-storchaka
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka commented Aug 16, 2022 •
edited by miss-islington
Loading

It should be created before calling the setUp() method, but after
checking for skipping a test.

Automerge-Triggered-By: GH:tiran

@serhiy-storchaka
pythongh-95736: Fix event loop creation in IsolatedAsyncioTestCase
1194f25 
It should be created before calling the setUp() method, but after
checking for skipping a test.
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.11 only security fixes labels Aug 16, 2022
@serhiy-storchaka serhiy-storchaka added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Aug 16, 2022

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 1194f25 🤖

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 Aug 16, 2022
@tiran
Copy link
Copy Markdown
Member

tiran commented Aug 16, 2022

I have a new ticket and PR GH-96031

@gvanrossum
Copy link
Copy Markdown
Member

gvanrossum commented Aug 16, 2022

Well, this fix sure looks simpler than @tiran's. I'll defer judgment until I've had time to look at things more thoroughly though.

@serhiy-storchaka serhiy-storchaka mentioned this pull request Aug 16, 2022
Closed
@tiran
Copy link
Copy Markdown
Member

tiran commented Aug 16, 2022

Let's merge this PR to unblock and fix main branch. We can solve #95736 and correct the problematic implementation in the async unittest helper later.

@gvanrossum
Copy link
Copy Markdown
Member

gvanrossum commented Aug 16, 2022

The wasm32-wasi PR build fails with a different error:

======================================================================
FAIL: test_errno_mapping (test.test_exception_hierarchy.HierarchyTest.test_errno_mapping)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Lib/test/test_exception_hierarchy.py", line 94, in test_errno_mapping
    self.assertIs(type(e), OSError)
AssertionError: <class 'PermissionError'> is not <class 'OSError'>
----------------------------------------------------------------------

This was referenced Aug 16, 2022
Closed
Closed
@gvanrossum
Copy link
Copy Markdown
Member

gvanrossum commented Aug 17, 2022

Tho other WASM builds are fine. @tiran what do you think?

gvanrossum
gvanrossum approved these changes Aug 17, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Assuming the failing buildbots are explained this LGTM.

@tiran tiran added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 17, 2022
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Aug 17, 2022

🤖 New build scheduled with the buildbot fleet by @tiran for commit ef7e7f6 🤖

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 Aug 17, 2022
@tiran
Copy link
Copy Markdown
Member

tiran commented Aug 17, 2022

The wasm32-wasi PR build fails with a different error:

It's an unrelated error which is already fixed in main.

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Aug 17, 2022

@serhiy-storchaka: Status check is done, and it's a pending ❌ .

1 similar comment
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Aug 17, 2022

@serhiy-storchaka: Status check is done, and it's a pending ❌ .

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Aug 17, 2022

Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 3651710 into python:main Aug 17, 2022
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Aug 17, 2022

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Aug 17, 2022

Sorry @serhiy-storchaka, I had trouble checking out the 3.11 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 36517101dd80cae93da379e95e98a688c52935b7 3.11

@miss-islington miss-islington self-assigned this Aug 17, 2022
@kumaraditya303 kumaraditya303 added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Aug 17, 2022
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Aug 17, 2022

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Aug 17, 2022
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Aug 17, 2022

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 17, 2022
@serhiy-storchaka @miss-islington
pythongh-95736: Fix event loop creation in IsolatedAsyncioTestCase (p…
5e45a4c 
…ythonGH-96033)

It should be created before calling the setUp() method, but after
checking for skipping a test.

Automerge-Triggered-By: GH:tiran
(cherry picked from commit 3651710)

Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiran tiran tiran approved these changes

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees

@miss-islington miss-islington

Labels

skip news type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@serhiy-storchaka @bedevere-bot @tiran @gvanrossum @miss-islington @kumaraditya303