Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
Improve tests for str to Fraction conversion#134010
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| self.assertEqual((3, 2), _components(F("3/2"))) | ||
| self.assertEqual((3, 2), _components(F("3 / 2"))) | ||
| self.assertEqual((3, 2), _components(F(" \n +3/2"))) | ||
| self.assertEqual((-3, 2), _components(F("-3/2 "))) | ||
| self.assertEqual((13, 2), _components(F("013/02 \n "))) | ||
| self.assertEqual((13, 2), _components(F("0013/002 \n "))) |
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.
Why this was replaced, not added?
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.
It is virtually the same test, but tests that more than one zero is accepted. There were no tests for leading zero in other forms, so they were added.
| def check_invalid(s): | ||
| msg = "Invalid literal for Fraction: " + repr(s) | ||
| self.assertRaisesMessage(ValueError, msg, F, s) |
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 looks fine.
But maybe use subTest?
forvaluein ["3/","/2",# there can be a lot of...# Denominators don't need a sign."3/+2","3/-2",# Imitate float's parsing. ...]:withself.subTest(value=value):check_invalid(value)
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.
The main reason is that if we make error messages more specific, it is easier to add an optional parameter, while keeping default for most tests. For example, I originally planned to add the tests here that are currently in test_limit_int.
17d0fec intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
(cherry picked from commit17d0fec)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
(cherry picked from commit17d0fec)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-134016 is a backport of this pull request to the3.14 branch. |
GH-134017 is a backport of this pull request to the3.13 branch. |
No description provided.