Skip to content

gh-126004: remove redundant safeguards for codecs handlers tests - #127680

Merged
picnixz merged 5 commits into
python:mainfrom
picnixz:fix/codecs/safe-handlers-126004
Jan 23, 2025
Merged

gh-126004: remove redundant safeguards for codecs handlers tests#127680
picnixz merged 5 commits into
python:mainfrom
picnixz:fix/codecs/safe-handlers-126004

Conversation

@picnixz

@picnixzpicnixz commented Dec 6, 2024•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-appbedevere-appBot mentioned this pull request Dec 6, 2024
Closed
@bedevere-appbedevere-appBot added tests Tests in the Lib/test dir awaiting review labels Dec 6, 2024
encukou
encukou reviewed Dec 6, 2024
View reviewed changes
Comment threadLib/test/test_capi/test_codecs.py
@picnixz
picnixz requested a review from encukouDecember 28, 2024 10:17
@encukou

encukou commented Jan 6, 2025

Copy link
Copy Markdown
Member

I'd prefer merging this after the 3 fixes; at that point this would add additional assertions and remove the safe argument.

@picnixzpicnixz changed the title gh-126004: allow to mark codecs handlers tests as safegh-126004: remove safeguards for crashing codecs handlersJan 23, 2025
@picnixzpicnixz changed the title gh-126004: remove safeguards for crashing codecs handlersgh-126004: remove redundant safeguards for codecs handlersJan 23, 2025
@picnixzpicnixz changed the title gh-126004: remove redundant safeguards for codecs handlersgh-126004: remove redundant safeguards for codecs handlers testsJan 23, 2025
@picnixz
picnixz requested a review from vstinnerJanuary 23, 2025 13:57
vstinner
vstinner approved these changes Jan 23, 2025
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@picnixz
picnixz merged commit cf0b2da into python:mainJan 23, 2025
@picnixz
picnixz deleted the fix/codecs/safe-handlers-126004 branch January 23, 2025 15:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes
@encukouencukouAwaiting requested review from encukou

Assignees

No one assigned

Labels

skip newstestsTests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@picnixz@encukou@vstinner