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 — bpo-46679: Don't ignore timeout argument in test.support.wait_process. by notarealdeveloper · Pull Request #31205 · python/cpython

Conversation

@notarealdeveloper

@notarealdeveloper notarealdeveloper commented Feb 7, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

The function wait_process in Lib/test/support/__init__.py ignores its timeout argument. This argument is useful, for example, in tests that need to determine whether a deadlock has been fixed (e.g., the test added in #30310).

https://bugs.python.org/issue46679

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
notarealdeveloper added 2 commits February 1, 2022 11:12
@notarealdeveloper
Fix bug in timeout kwd of test.support.wait_process
12a5562
@notarealdeveloper
Add NEWS.d entry.
fedb85f
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 7, 2022
@bedevere-bot bedevere-bot added the awaiting review label Feb 7, 2022
@notarealdeveloper notarealdeveloper mentioned this pull request Feb 7, 2022
bpo-46210: Fix deadlock in print. #30310
Closed
sobolevn
sobolevn reviewed Feb 11, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Tests/2022-02-07-13-24-28.bpo-46679.S-HI_R.rst Outdated
@@ -0,0 +1,2 @@
The function ``wait_process`` in ``Lib/test/support/__init__.py`` no longer

@sobolevn sobolevn Feb 11, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

NEWS are not required for test-only changes. We need them for user-facing changes only 🙂

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@notarealdeveloper notarealdeveloper Feb 14, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good to know! Thanks! :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@AlexWaygood AlexWaygood added the skip news label Feb 11, 2022
@sobolevn sobolevn mentioned this pull request Feb 11, 2022
bpo-46711: increase timeout for test_logging::test_post_fork_child_no_deadlock #31274
Closed
@AlexWaygood AlexWaygood added the tests Tests in the Lib/test dir label Feb 11, 2022
@notarealdeveloper
Remove NEWS.d entry.
40dc1cc
@notarealdeveloper

notarealdeveloper commented May 23, 2022

Copy link
Copy Markdown
Contributor Author

@kumaraditya303 Done. :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@MaxwellDupre

MaxwellDupre commented Oct 1, 2022

Copy link
Copy Markdown
Contributor

The wait_process function has changed significantly since your changes in the latest 3.12 could you please check?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303
kumaraditya303 requested review from vstinner and removed request for vstinner October 1, 2022 17:24
@vstinner

vstinner commented Oct 3, 2022

Copy link
Copy Markdown
Member

I fixed this issue with commit 0ba8027. But IMO it's still interesting to fix this typo in Python 3.10 and 3.11. Can you please create a PR for the 3.11 branch?
All reactions

Sorry, something went wrong.

Uh oh!

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

kumaraditya303 commented Nov 29, 2022

Copy link
Copy Markdown
Contributor

Closing as it is fixed on main. If you are interested, feel free to create 3.11 backport as Victor suggested.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303 kumaraditya303 closed this Nov 29, 2022
@vstinner

vstinner commented Nov 29, 2022

Copy link
Copy Markdown
Member

This issue got fixed in 3.11 by commit a9a8c87 and backported to 3.10 as commit ae5317d: see issue gh-90867.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@notarealdeveloper notarealdeveloper mannequin mentioned this pull request Apr 10, 2022
test.support.wait_process ignores timeout argument #90837
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn left review comments

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review 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.

9 participants

@notarealdeveloper @MaxwellDupre @vstinner @kumaraditya303 @sobolevn @the-knights-who-say-ni @ezio-melotti @bedevere-bot @AlexWaygood
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.