-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-29773: Add more cases for testing string to float conversion errors. #580
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
bpo-29773: Add more cases for testing string to float conversion errors. #580
Conversation
@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tiran, @zooba, @mdickinson, @benjaminp and @birkenfeld to be potential reviewers. |
Lib/test/test_float.py
Outdated
# non-UTF-8 byte string | ||
check(b'123\xbd') | ||
# lone surrogate in Unicode string | ||
#check('123\udca0') |
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 dislike commented test. Either fix it to catch UnicodeEncodeError (which would be a CPython implementation specific test), or remove it.
Maybe it's even worth it to enhance the error message for that one.
Lib/test/test_float.py
Outdated
check(b' 123 456 ') | ||
|
||
# non-ascii digits | ||
check('\u0663\u0661\u0664!') |
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 suggest '\u0663\u0661\u0664' + '!', at the first read, I understood that the error came from non-ASCII digits. Or explain that the error is on the invalid '!' character.
Lib/test/test_float.py
Outdated
# byte string with embedded NUL | ||
check(b'123\x00') | ||
# non-UTF-8 byte string | ||
check(b'123\xbd') |
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.
Maybe it's worth it to factorize tests which use the "same" text and bytes string (text encoded to Latin1): all tests except of '\u0663\u0661\u0664!', no?
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 prefer to be more explicit here.
…rs. (python#580) (cherry picked from commit 9e6ac83)
…rs. (python#580) (cherry picked from commit 9e6ac83)
Bumps [sentry-sdk](https://github.com/getsentry/sentry-python) from 1.9.6 to 1.9.9. - [Release notes](https://github.com/getsentry/sentry-python/releases) - [Changelog](https://github.com/getsentry/sentry-python/blob/master/CHANGELOG.md) - [Commits](getsentry/sentry-python@1.9.6...1.9.9) --- updated-dependencies: - dependency-name: sentry-sdk dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
No description provided.