Skip to content

gh-131015: Add test for bytes formatting errors #131881

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

Merged
merged 5 commits into from
Apr 5, 2025

Conversation

ApostolFet
Copy link
Contributor

@ApostolFet ApostolFet commented Mar 29, 2025


pi = PseudoFloat(3.1415)

self.assertRaisesRegex(TypeError, '%x format: an integer is required, not float', operator.mod, '%x', 3.14)
Copy link
Member

Choose a reason for hiding this comment

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

Can you make it so that all lines are under 80 chars please? I think the existing code is breaking that limit but I'm not entirely sure. More generally, you could do mod = operator.mod to reduce the overall line length as well.

mod = operator.mod
msg = "...."
self.assertRaisesRegex(TypeError, msg, mod, ...)

Or use a private helper for this test.

Copy link
Contributor Author

@ApostolFet ApostolFet Apr 4, 2025

Choose a reason for hiding this comment

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

What do you think about that option?

        exceptions_params = (
            ('%x format: an integer is required, not float','%x', 3.14),
            ...
        )

        for msg, format, value in exceptions_params:
            self.assertRaisesRegex(TypeError, msg , mod, format, value)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, something like this is fine. It'll become easier to add more test cases later.

Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
picnixz
picnixz previously approved these changes Apr 4, 2025
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

It could have been possible to also separate the error checks and put them in a separate method but the other tests don't seem to separate the good and the bad cases so let's keep it that way.

@picnixz picnixz dismissed their stale review April 4, 2025 18:16

We should test bytes and not strings formatting.

@picnixz picnixz self-assigned this Apr 5, 2025
@picnixz
Copy link
Member

picnixz commented Apr 5, 2025

I'll merge this tomorrow.

@picnixz picnixz merged commit 0555778 into python:main Apr 5, 2025
42 checks passed
@picnixz picnixz added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Apr 5, 2025
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

Sorry, @ApostolFet and @picnixz, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 05557788f3c284ede73e6f94810ec796bb9d3721 3.12

@miss-islington-app
Copy link

Sorry, @ApostolFet and @picnixz, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 05557788f3c284ede73e6f94810ec796bb9d3721 3.13

@picnixz
Copy link
Member

picnixz commented Apr 5, 2025

I'll make the backports

@bedevere-app
Copy link

bedevere-app bot commented Apr 5, 2025

GH-132114 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Apr 5, 2025
@bedevere-app
Copy link

bedevere-app bot commented Apr 5, 2025

GH-132115 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Apr 5, 2025
picnixz added a commit to picnixz/cpython that referenced this pull request Apr 5, 2025
Co-authored-by: Bénédikt Tran <[email protected]>
(cherry picked from commit 0555778)
picnixz added a commit that referenced this pull request Apr 5, 2025
…2115)

* gh-131015: Add test for bytes formatting errors (#131881)

Co-authored-by: Ageev Maxim <[email protected]>
(cherry picked from commit 0555778)
picnixz added a commit that referenced this pull request Apr 5, 2025
…2114)

* gh-131015: Add test for bytes formatting errors (#131881)

Co-authored-by: Ageev Maxim <[email protected]>
(cherry picked from commit 0555778)
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants