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 fixes: fix record-test.sh for current fonttools, doc update #445

Closed
wants to merge 5 commits into from
Closed

Test fixes: fix record-test.sh for current fonttools, doc update #445

wants to merge 5 commits into from

Conversation

domschl
Copy link
Contributor

@domschl domschl commented Mar 26, 2017

This introduces changes to the test-tools and a documentation update:

  • record-test.sh is incompatible with current (3.9.1) fonttools, the subset-font-file is now called font.subset.ttf (instead of font.ttf.subset), this updates for the new fonttools behavior.
  • Documentation: record_it.sh is now called record_test.sh

(Former change of shebangs is reverted, see discussion below)

@khaledhosny
Copy link
Collaborator

since the tools won't work with python3 on platforms that have python3 as default python.

They worked for me rather recently, what part did not work with Python 3, we should probably just fix it.

@domschl
Copy link
Contributor Author

domschl commented Mar 26, 2017

Running the existing tests does work without any changes, however the test-recording tools do not.
Example of a failure with python3:

$ ./hb-unicode-encode F68 F71 F74 | ./record-test.sh ../../util/hb-shape DDC_Uchen.ttf

File "./hb-unicode-decode", line 5, in <module> UtilMains.filter_multiple_strings_or_stdin (Unicode.decode, "UNICODE_STRING") File ".../git/Tibetan/harfbuzz/test/shaping/hb_test_tools.py", line 432, in filter_multiple_strings_or_stdin print (callback (line)) File ".../git/Tibetan/harfbuzz/test/shaping/hb_test_tools.py", line 448, in decode return u','.join ("U+%04X" % ord (u) for u in unicode (s, 'utf-8')).encode ('utf-8') NameError: name 'unicode' is not defined

It would be some work to fix this for both python2 and 3 due to differences in unicode handling between the two versions.

@khaledhosny
Copy link
Collaborator

We are almost Python 3 compatible, let me fix the small remaining bit.

khaledhosny added a commit that referenced this pull request Mar 26, 2017
@khaledhosny
Copy link
Collaborator

The Python 3 part should be fixed now.

@domschl
Copy link
Contributor Author

domschl commented Mar 26, 2017

Hi, your changes work fine, and this is a much better solution, so I've undone the insertion of the shebangs. Is that the right approach, or should I generate a new pull request?

@domschl domschl changed the title Test fixes: fix record-test.sh for current fonttools, explicit python2 shebang, doc update Test fixes: fix record-test.sh for current fonttools, doc update Mar 26, 2017
@khaledhosny
Copy link
Collaborator

You can use interactive rebase git rebase -i to drop the unwanted commits, or you can create a new pull request. Whatever works for you.

@domschl
Copy link
Contributor Author

domschl commented Mar 27, 2017

Git took already care of it. The patch doesn't affect the python-files anymore. Tx!

@khaledhosny
Copy link
Collaborator

But there are still 5 commit when there should only be 2. Fixed it and pushed manually.

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