Skip to content
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

better rect tests using literal values #3190

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AntoineMamou
Copy link

Fix the following functions in rect_test to use literal values for attribute checks : test_inflate__larger, test_inflate__smaller, test_inflate_ip__larger and test_inflate_ip__smaller.

@AntoineMamou AntoineMamou requested a review from a team as a code owner October 23, 2024 10:11
@bilhox bilhox linked an issue Oct 23, 2024 that may be closed by this pull request
@bilhox bilhox changed the title fixing rec_test functions on this issue https://github.com/pygame-community/pygame-ce/issues/3096 better rect tests using literal values Oct 23, 2024
@bilhox
Copy link
Contributor

bilhox commented Oct 23, 2024

Hi antoine, since you're new to contributing, let me give you one little advice on how you can improve your PRs next time. You don't have to mention the link of the issue in the title, you can mention it in your message directly. With this, you can opt for a more descriptive title for your PR.

@yunline yunline added tests tests (module) Code quality/robustness Code quality and resilience to changes labels Oct 24, 2024
@bilhox
Copy link
Contributor

bilhox commented Oct 31, 2024

I think there is a little quiproquo, when I talked about tests for literal values, I meant for rect attributes, not these ones.

@AntoineMamou
Copy link
Author

Hello @bilhox, I think I don't really understand what should be done. Could you explain more specifically, maybe ?

Comment on lines +794 to +800
self.assertEqual((5, 8), r2.center)
self.assertEqual(4, r2.left)
self.assertEqual(7, r2.top)
self.assertEqual(6, r2.right)
self.assertEqual(9, r2.bottom)
self.assertEqual(2, r2.width)
self.assertEqual(2, r2.height)
Copy link
Contributor

@gresm gresm Nov 6, 2024

Choose a reason for hiding this comment

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

Some whitespace snuck by. I recommend using autoformatting tools to fix that.
You can either:

pip install pre-commit
pre-commit run -a
pre-commit install  # completely optional, will cause the pre-commit to run every time you make a commit.

Or

python dev.py format

Both I recommend running in a virtualenv.

@bilhox
Copy link
Contributor

bilhox commented Nov 7, 2024

Hello @bilhox, I think I don't really understand what should be done. Could you explain more specifically, maybe ?

What I meant is we should have a test similar to testCalculatedAttributes that check attributes values based on literal values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality/robustness Code quality and resilience to changes tests tests (module)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect tests & missing test in rect_test
5 participants