← 返回首页
gh-94808: Add coverage for bytesarray_setitem by mdboom · Pull Request #95802 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

gh-94808: Add coverage for bytesarray_setitem#95802

Merged
JelleZijlstra merged 1 commit into
python:mainfrom
mdboom:coverage-bytesarray-setitem
Oct 10, 2022
Merged

gh-94808: Add coverage for bytesarray_setitem#95802
JelleZijlstra merged 1 commit into
python:mainfrom
mdboom:coverage-bytesarray-setitem

Conversation

mdboom commented Aug 8, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

When both are provided, tp_ass_subscript takes precedence over tp_ass_item. Since bytesarray provides both, the existing test_setitem tests for bytesarray were not testing bytesarray_setitem, but bytesarray_ass_subscript. This is mostly fine, since Python code has to jump through some hoops to even call it, but a third-party library using PySequence_SetItem could potentially run into this uncovered case.

mdboom requested a review from brandtbucher August 8, 2022 19:45
mdboom mentioned this pull request Aug 8, 2022
Open
mdboom added the tests Tests in the Lib/test dir label Aug 11, 2022
mdboom closed this Aug 12, 2022
mdboom reopened this Aug 12, 2022
mdboom force-pushed the coverage-bytesarray-setitem branch from 5f24775 to e717c5c Compare August 12, 2022 18:11
mdboom force-pushed the coverage-bytesarray-setitem branch from e717c5c to 1c2a344 Compare October 3, 2022 21:20
JelleZijlstra approved these changes Oct 7, 2022
View reviewed changes

JelleZijlstra 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Looks good. Any reason you didn't mark this for backporting to 3.10?

JelleZijlstra self-assigned this Oct 7, 2022

mdboom commented Oct 10, 2022

Copy link
Copy Markdown
Contributor Author

Looks good. Any reason you didn't mark this for backporting to 3.10?

No real reason not to -- for all these other coverage improvements linked to #94808, we've just been backporting to 3.11.

JelleZijlstra added the needs backport to 3.10 only security fixes label Oct 10, 2022
JelleZijlstra merged commit dfcdee4 into python:main Oct 10, 2022

miss-islington commented Oct 10, 2022

Copy link
Copy Markdown
Contributor

Thanks @mdboom for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

miss-islington commented Oct 10, 2022

Copy link
Copy Markdown
Contributor

Sorry, @mdboom and @JelleZijlstra, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker dfcdee4a18ece6f7c4fe1aa5830ee861f8b15b24 3.11

miss-islington commented Oct 10, 2022

Copy link
Copy Markdown
Contributor

Sorry @mdboom and @JelleZijlstra, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker dfcdee4a18ece6f7c4fe1aa5830ee861f8b15b24 3.10

JelleZijlstra commented Oct 10, 2022

Copy link
Copy Markdown
Member

Thanks! I think it's good to backport new tests to the bugfix branches so we can be more confident in any future bugfixes. But testing on 3.11 is definitely more important.

Would you mind doing the manual backports? Feel free to skip 3.10 if you don't think it's worth it.

mpage pushed a commit to mpage/cpython that referenced this pull request Oct 11, 2022
pythongh-94808: Add coverage for bytesarray_setitem (python#95802)
fbfd13b
mdboom deleted the coverage-bytesarray-setitem branch December 22, 2022 15:50
ZeroIntensity removed needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Feb 17, 2025
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

JelleZijlstra JelleZijlstra approved these changes
brandtbucher Awaiting requested review from brandtbucher

Assignees

JelleZijlstra

Labels

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.

5 participants

Footer

© 2026 GitHub, Inc.