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

Run Apple tests in CI #113

Merged
merged 12 commits into from
Jun 22, 2024
Merged

Run Apple tests in CI #113

merged 12 commits into from
Jun 22, 2024

Conversation

LaurenzV
Copy link
Collaborator

Nearly done, just one failing test case I need to investigate.

@LaurenzV
Copy link
Collaborator Author

LaurenzV commented Jun 22, 2024

@behdad Do you know whether it's a bug or "intended" that the following two commands yield different results?

hb-shape "/System/Library/Fonts/Supplemental/Devanagari Sangam MN.ttc" --unicodes U+092D,U+0941 --no-glyph-names
[160=0+1339|1324=0@-296,11+0]
hb-shape "/System/Library/Fonts/Supplemental/Devanagari Sangam MN.ttc" --unicodes U+092D,U+0941 --shaper=coretext --no-glyph-names
[160=0+1043|1324=0@0,11+296]

I'm not sure if passing --shaper=coretext is always supposed to have the same results.

EDIT: Ah, I guess while the numbers are different, the visual result itself should be the same.

@RazrFalcon
Copy link
Collaborator

So we did had one bug in ot_shape.rs, still way better then I was expecting. But I guess this code haven't changed since I last tested it.

Having our copy of macos.tests is meh. Can't we simply add some tests to the ignore list?
And you have already added hash "support" to the Python script.

@LaurenzV
Copy link
Collaborator Author

So we did had one bug in ot_shape.rs, still way better then I was expecting. But I guess this code haven't changed since I last tested it.

Two more bugs actually. But yes, luckily not a lot of bugs.

Having our copy of macos.tests is meh. Can't we simply add some tests to the ignore list?

The thing is that some of the font paths changed apparently, meaning that we would lose many tests just because the font path changed, which would be kind of a waste... I don't think macos.tests changes often, so I hope it shouldn't be a big deal. It's not ideal, but certainly better than having no tests at all!

And you have already added hash "support" to the Python script.

Well, right now we basically ignore them, because as mentioned in the file I don't think we need to bother with running on different MacOS versions, we always run on MacOS 14 for now, and we use the output from harfbuzz as our test oracle. I think that is the cleanest approach for now.

@LaurenzV
Copy link
Collaborator Author

Alright, it works now. :D So we can merge this after harfbuzz/ttf-parser#157 is merged.

@LaurenzV LaurenzV marked this pull request as ready for review June 22, 2024 10:44
@RazrFalcon
Copy link
Collaborator

Ok, makes sense.

@RazrFalcon RazrFalcon merged commit d184cf3 into harfbuzz:master Jun 22, 2024
2 checks passed
@LaurenzV LaurenzV deleted the apple-ci branch June 22, 2024 10:45
@LaurenzV
Copy link
Collaborator Author

Nice! I'll attempt to port the aat changes myself next, hopefully it will work.

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.

2 participants