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.

Sign up for GitHub

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

Jump to bottom

Try getting freetype tests running on CI, fix minor test fail issues #2370

Merged
merged 3 commits into from
Aug 5, 2023
Merged

Try getting freetype tests running on CI, fix minor test fail issues #2370

merged 3 commits into from
Aug 5, 2023

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Aug 1, 2023

No description provided.

@ankith26 ankith26 requested a review from a team as a code owner August 1, 2023 17:19
@dr0id
Copy link
Contributor

dr0id commented Aug 1, 2023

The freetype test 'test_freetype_Font_name' is expected to fail with current code, working on a fix right now.

@yunline yunline added tests tests (module) freetype pygame.freetype labels Aug 2, 2023
Starbuck5
Starbuck5 approved these changes Aug 2, 2023
View reviewed changes
Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

Thanks!

@Starbuck5
Copy link
Member

Starbuck5 commented Aug 2, 2023 •
edited
Loading

This PR is good, but should we have the behavior change in here at all?

It's from #2175

It changes freetype.Font.name to raise an AttributeError on un-initialized freetype.Fonts. This changes the behavior, the change is not documented, and breaks the test now that it runs.

@dr0id
Copy link
Contributor

dr0id commented Aug 2, 2023

This PR is good, but should we have the behavior change in here at all?

It's from #2175

It changes freetype.Font.name to raise an AttributeError on un-initialized freetype.Fonts. This changes the behavior, the change is not documented, and breaks the test now that it runs.

It was a PR-feedback that suggested to change that since 'repr(font)' is confusing when calling f.name. The documentation for that is missing, I agree. But honestly, how many times did you encounter an uninitialized font in your coding experience with pygame/pygame-ce?

@dr0id
Copy link
Contributor

dr0id commented Aug 2, 2023

Here is the follow up PR to catch all uninitialized fonts:
#2371

@ankith26 ankith26 changed the title Try getting freetype tests running on CI Try getting freetype tests running on CI, fix minor test fail issues Aug 5, 2023
@Starbuck5 Starbuck5 merged commit 7837635 into main Aug 5, 2023
@Starbuck5 Starbuck5 added this to the 2.4.0 milestone Aug 5, 2023
@ankith26 ankith26 deleted the ankith26-freetype-ci branch August 5, 2023 05:52
@ankith26 ankith26 restored the ankith26-freetype-ci branch August 5, 2023 18:40
@ankith26 ankith26 mentioned this pull request Aug 5, 2023
Merged
ankith26 added a commit that referenced this pull request Aug 5, 2023
@ankith26
Merge pull request #2383 from pygame-community/ankith26-freetype-ci
9a5ac27 
[BACKPORT OF #2370] Try getting freetype tests running on CI, fix minor test fail issues
@ankith26 ankith26 deleted the ankith26-freetype-ci branch August 5, 2023 18:43
@ankith26 ankith26 modified the milestone: 2.4.0 Aug 10, 2023
@ankith26 ankith26 modified the milestones: 2.4.0, 2.3.2 Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr0id dr0id dr0id approved these changes

@Starbuck5 Starbuck5 Starbuck5 approved these changes

Assignees
No one assigned
Labels
freetype pygame.freetype tests tests (module)
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ankith26 @dr0id @Starbuck5 @yunline