Skip to content

gh-103272: add regression test for getattr that raises#103336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
hauntsaninja merged 6 commits into from
Apr 7, 2023
Merged

gh-103272: add regression test for getattr that raises #103336

hauntsaninja merged 6 commits into from
Apr 7, 2023

Conversation

@sunmy2019
Copy link
Member

@sunmy2019sunmy2019 commented Apr 7, 2023

Add test case taken from gh-103272 for #103332.

@bedevere-botbedevere-bot added the tests Tests in the Lib/test dir label Apr 7, 2023
@bedevere-bot
Copy link

bedevere-bot commented Apr 7, 2023

Most changes to Python require a NEWS entry.

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

@bedevere-botbedevere-bot mentioned this pull request Apr 7, 2023
Closed
hauntsaninja
hauntsaninja reviewed Apr 7, 2023
View reviewed changes
Copy link
Contributor

@hauntsaninjahauntsaninja left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Awesome work, thanks for narrowing this down!

I think we can make the test case slightly simpler to make it clearer what's getting tested:

class A: def __getattr__(self, name): raise ValueError @property def foo(self): return self.__getattr__("asdf") A().foo

And maybe rename the test to test_getattr_raises or something

@hauntsaninjahauntsaninja changed the title add test case for gh-103272gh-103272: add regression testApr 7, 2023
@hauntsaninjahauntsaninja changed the title gh-103272: add regression testgh-103272: add regression test for getattr that raisesApr 7, 2023
@sobolevn
Copy link
Member

sobolevn commented Apr 7, 2023

I've got the revert merged, now this test must pass :)

sobolevn
sobolevn reviewed Apr 7, 2023
View reviewed changes
Copy link
Member

@sobolevnsobolevn left a comment

Choose a reason for hiding this comment

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

Thank you!

sobolevn
sobolevn reviewed Apr 7, 2023
View reviewed changes
sobolevn
sobolevn approved these changes Apr 7, 2023
View reviewed changes
Copy link
Member

@sobolevnsobolevn left a comment

Choose a reason for hiding this comment

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

Congrats on your first CPython PR 🎉

hauntsaninja
hauntsaninja approved these changes Apr 7, 2023
View reviewed changes
Copy link
Contributor

@hauntsaninjahauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks again, this is great!

@hauntsaninjahauntsaninja added the needs backport to 3.11 only security fixes label Apr 7, 2023
@hauntsaninjahauntsaninja merged commit 5d7d86f into python:mainApr 7, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Apr 7, 2023

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 7, 2023
@sunmy2019@miss-islington
pythongh-103272: regression test for getattr exception in property (p…
29824ef
…ythonGH-103336) (cherry picked from commit 5d7d86f) Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>
@bedevere-bot
Copy link

bedevere-bot commented Apr 7, 2023

GH-103351 is a backport of this pull request to the 3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11 only security fixes label Apr 7, 2023
miss-islington added a commit that referenced this pull request Apr 7, 2023
@miss-islington@sunmy2019
gh-103272: regression test for getattr exception in property (GH-103336)
b8d1623
(cherry picked from commit 5d7d86f) Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@sunmy2019@warsaw
pythongh-103272: regression test for getattr exception in property (p…
a82048a
…ython#103336)
@AlexWaygoodAlexWaygood mentioned this pull request Apr 14, 2023
Closed
@sunmy2019sunmy2019 mentioned this pull request Apr 25, 2023
Merged
@sunmy2019sunmy2019 deleted the test-case-for-gh-103272 branch May 10, 2023 14:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevnsobolevnsobolevn approved these changes

@hauntsaninjahauntsaninjahauntsaninja approved these changes

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.

6 participants

@sunmy2019@bedevere-bot@sobolevn@miss-islington@hauntsaninja@arhadthedev