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

Test function for pygame.draw.arc (resolves #2636) #2638

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Houda222
Copy link

This pull request resolves issue #2636

I added a test function test_arc__correct_drawing that replaced the todo_test_arc function in the DrawArcMixin class. It ensures that pygame.draw.arc works correctly, by checking whether the drawn arc matches the ellipse defined by the bounding rectangle. The comparison is done by measuring the difference between the ellipse's points and the arc's point and making sure it's less than a certain threshold.
The test is done on different bounding rectangles.

@Houda222 Houda222 requested a review from a team as a code owner December 28, 2023 13:47
@ankith26
Copy link
Member

The circleci fail on this PR is not this PRs fault, don't worry. It's being resolved in another PR

self.assertEqual(
number_of_valid_arc_points,
number_of_invalid_arc_points + number_of_valid_arc_points,
)
Copy link
Member

Choose a reason for hiding this comment

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

this check can more simply be written as self.assertEqual(number_of_invalid_arc_points, 0).

Additionally, we should probably test for number_of_valid_arc_points being correct somehow?

@ankith26 ankith26 linked an issue Jan 6, 2024 that may be closed by this pull request
1 task
Comment on lines +6173 to +6174
start = 0
stop = 2
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we should probably test a broader range of arcs than just 0 to 2 radians, probably stuff like: pi, pi/2, pi/4, 2 * pi.

"rect": rect,
"start_angle": start,
"stop_angle": stop,
"width": 1,
Copy link
Member

Choose a reason for hiding this comment

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

Probably should also test on wider range of widths than just 1.

test/draw_test.py Outdated Show resolved Hide resolved
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

What is here is nice, but I think this test should test more angles and more widths to fully test out draw arc.

@MyreMylar MyreMylar closed this May 23, 2024
@MyreMylar MyreMylar reopened this May 23, 2024
@MyreMylar MyreMylar requested a review from a team as a code owner May 25, 2024 20:09
@MyreMylar
Copy link
Member

Hi @Houda222, do you think you will make any of the changes asked for? Or should we pass this PR over to someone else to finish off?

@ankith26 ankith26 marked this pull request as draft July 8, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Test for pygame.draw.arc in draw_test.py
3 participants