-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-130428: Add tests for delattr suggestions #130455
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.
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
base: main
Are you sure you want to change the base?
Conversation
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Lib/test/test_traceback.py
Outdated
actual = self.get_suggestion(lambda: delattr(obj, 'somethingverywrong')) | ||
self.assertNotIn("blech", actual) | ||
|
||
def test_delattr_error_bad_suggestions_do_not_trigger_for_small_names(self): |
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.
This test is the same as for getattr
, can't it be refactored so that we use the same fixtures but with different calls to self.get_suggestion
?
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.
I was actually thinking of doing that, but decided to go with duplication for better readability.
On reconsideration with your review, I believe it is redundant and should be refactored. Will update the PR accordingly.
@Pranjal095 Some of the tests you added are failing, could you fix them please? |
Let's put it as a draft until #130427 is merged then. |
Usually we'd merge a new feature together with its tests. Could you combine the PRs into one? |
…gestions-combined This merges the delattr suggestion feature implementation by @sobolevn from PR python#130427 with added tests from PR python#130455.
I've made a new PR combining the commits from both corresponding forks. Kindly let me know if there are any further modifications to be made. |
Depends on #130427.
With reference to the enhancement proposed in issue #130425, tests have been added to ensure proper functionality of delattr suggestions. To maintain consistency and readability of the code, they mirror the tests for getattr suggestions existing in the modified file (Lib/test/test_traceback.py).