Skip to content

Conversation

@johnslavik
Copy link
Member

@johnslavikjohnslavik commented Oct 18, 2025

This is generally a work in progress; tests are needed.
The asyncio patch is simple and ready.

@johnslavik
Copy link
MemberAuthor

Please merge #140298 first.

@johnslavikjohnslavik marked this pull request as ready for review October 23, 2025 14:57
@johnslavikjohnslavik marked this pull request as draft October 23, 2025 15:00
@johnslavik
Copy link
MemberAuthor

johnslavik commented Dec 16, 2025

Looks like I was confused about the regression -- that's good news!

This will have to wait until the loop is properly closed in the asyncio REPL.

@johnslavik
Copy link
MemberAuthor

Cherry-picked 92f629b (875fd2a) from GH-142785 to see if it fixes the problem.

@johnslavik
Copy link
MemberAuthor

to see if it fixes the problem.

It does! Cool!

Comment on lines +106 to +107
except SystemExit:
raise
Copy link
MemberAuthor

@johnslavikjohnslavikDec 20, 2025

Choose a reason for hiding this comment

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

I really don't like this, but it resembles the original behavior of the REPLs -- see GH-143023.
I don't think this is correct, but it's not super clear to me, so please chime in to GH-143023 to put your two cents in.

Suggested change
exceptSystemExit:
raise
# TODO: Revisit in GH-143023
exceptSystemExit:
raise

Copy link
MemberAuthor

@johnslavikjohnslavikDec 20, 2025

Choose a reason for hiding this comment

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

GH-143023 is also why I didn't add a test for this.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

We've got some good news, this is not bad! I'll add a test shortly.

@johnslavikjohnslavik marked this pull request as ready for review December 20, 2025 17:36
@johnslavik
Copy link
MemberAuthor

I'll see if I can simplify this. I think the tests are slightly overcomplicated.

@johnslavikjohnslavik marked this pull request as draft December 21, 2025 16:02
Copy link
MemberAuthor

@johnslavikjohnslavik left a comment

Choose a reason for hiding this comment

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

I'll come back to this after completing GH-140648.

@johnslavik
Copy link
MemberAuthor

I've realized that this isn't testing what it is supposed to test. I'll come back to this later.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@johnslavik