FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-104090: Fix unittest collectedDurations resources leak by bityob · Pull Request #106795 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-104090: Fix unittest collectedDurations resources leak - #106795

Merged
iritkatriel merged 4 commits into
python:mainfrom
bityob:gh-104090-fix-leaked-semaphors-on-test_concurrent_futures
Jul 19, 2023
ConversationCommits (4)ChecksFiles changed
Merged

gh-104090: Fix unittest collectedDurations resources leak#106795
iritkatriel merged 4 commits into
python:mainfrom
bityob:gh-104090-fix-leaked-semaphors-on-test_concurrent_futures

Conversation

bityob commented Jul 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor
  1. Fixed TestResult.addDuration method to add only test repr string and not the test object itself, to avoid resources leak

@iritkatriel

bedevere-bot commented Jul 15, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot mentioned this pull request Jul 15, 2023
Open
bityob force-pushed the gh-104090-fix-leaked-semaphors-on-test_concurrent_futures branch from cdbe1c4 to 8cd162f Compare July 16, 2023 08:53

iritkatriel commented Jul 16, 2023

Copy link
Copy Markdown
Member

I think it might be better to split this into two PRs (attached to the same issue), one for the unittest fix and the other for the resource tracker change.

bityob force-pushed the gh-104090-fix-leaked-semaphors-on-test_concurrent_futures branch from 5585599 to 509636a Compare July 16, 2023 11:58

bedevere-bot commented Jul 16, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bityob commented Jul 16, 2023

Copy link
Copy Markdown
Contributor Author

I think it might be better to split this into two PRs (attached to the same issue), one for the unittest fix and the other for the resource tracker change.

Done

Second PR: #106807

iritkatriel added tests Tests in the Lib/test dir stdlib Standard Library Python modules in the Lib/ directory labels Jul 16, 2023

iritkatriel commented Jul 16, 2023

Copy link
Copy Markdown
Member

CC unittest maintainers. @gpshead @ezio-melotti

See issue for motivation.

iritkatriel commented Jul 17, 2023

Copy link
Copy Markdown
Member

@giampaolo addDuration was added in 3.12. Should this be backported as a bugfix?

iritkatriel requested a review from giampaolo July 17, 2023 15:10

sunmy2019 commented Jul 18, 2023

Copy link
Copy Markdown
Member

Is this new entry necessary?

Should this be backported as a bugfix?

+1 from me

sunmy2019 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2023

bedevere-bot commented Jul 18, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @sunmy2019 for commit d493239 🤖

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

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2023
iritkatriel added the needs backport to 3.12 only security fixes label Jul 18, 2023
sunmy2019 mentioned this pull request Jul 19, 2023
Closed

giampaolo commented Jul 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@iritkatriel wrote:

@giampaolo addDuration was added in 3.12. Should this be backported as a bugfix?

Hi! Yes, I think it should be backported.

iritkatriel merged commit 70b961e into python:main Jul 19, 2023

miss-islington commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Thanks @bityob for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

bedevere-bot commented Jul 19, 2023

Copy link
Copy Markdown

GH-106888 is a backport of this pull request to the 3.12 branch.

bedevere-bot removed the needs backport to 3.12 only security fixes label Jul 19, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 19, 2023
pythongh-104090: Fix unittest collectedDurations resources leak (pyth…
8b1fdec 
…onGH-106795)

(cherry picked from commit 70b961e)

Co-authored-by: Yonatan Bitton <bityob@gmail.com>
iritkatriel added the needs backport to 3.12 only security fixes label Jul 19, 2023

miss-islington commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Thanks @bityob for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 19, 2023
pythongh-104090: Fix unittest collectedDurations resources leak (pyth…
e0a4576 
…onGH-106795)

(cherry picked from commit 70b961e)

Co-authored-by: Yonatan Bitton <bityob@gmail.com>
iritkatriel pushed a commit that referenced this pull request Jul 19, 2023
[3.12] gh-104090: Fix unittest collectedDurations resources leak (GH-…
b1c50b8 
…106795) (#106888)

gh-104090: Fix unittest collectedDurations resources leak (GH-106795)
(cherry picked from commit 70b961e)

Co-authored-by: Yonatan Bitton <bityob@gmail.com>

gpshead commented Jul 19, 2023

Copy link
Copy Markdown
Member

thanks for figuring this out!

picnixz removed the needs backport to 3.12 only security fixes label Sep 14, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

iritkatriel iritkatriel approved these changes
giampaolo Awaiting requested review from giampaolo

Assignees

No one assigned

Labels

stdlib Standard Library Python modules in the Lib/ directory tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL