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

[subset] No need to keep actual expected output in tree; just the hash #3118

Closed
behdad opened this issue Aug 4, 2021 · 5 comments
Closed
Assignees
Labels
subset hb-subset related bugs tests

Comments

@behdad
Copy link
Member

behdad commented Aug 4, 2021

So. We keep the expected output, then compare hashes. That's slower than just comparing the files directly.

That said, I suggest we remove expected binaries and make the .tests files just list the expected hash. If hash mismatch happens, we suggest regenerating and while doing that we compare TTX to fonttools to approve of the generated output.

That, or remove the hashing.

@khaledhosny
Copy link
Collaborator

We can remove the hashing and just compare the file content, but I’m worried about removing expectation from the repo as they help in debugging CI failures that are not reproducible locally that happen from time to time by being able to see the ttx diff.

@khaledhosny khaledhosny added subset hb-subset related bugs tests labels Aug 5, 2021
@behdad
Copy link
Member Author

behdad commented Aug 5, 2021

We can remove the hashing and just compare the file content, but I’m worried about removing expectation from the repo as they help in debugging CI failures that are not reproducible locally that happen from time to time by being able to see the ttx diff.

Sure. We can do that for now. When things are much more stable we can reconsider.

@khaledhosny
Copy link
Collaborator

I tried comparing file contents instead of hashes, and I’m not seeing any measurable difference, so I’m inclined to leave the code as it until we later store hashes instead of the actual files.

@behdad
Copy link
Member Author

behdad commented Aug 6, 2021

SGTM.

@behdad behdad closed this as completed Aug 6, 2021
@behdad
Copy link
Member Author

behdad commented Aug 6, 2021

On second thought, it's just wrong to check hashes when we have both actual and expected readily. :)

@behdad behdad reopened this Aug 6, 2021
@behdad behdad closed this as completed in b3f8288 Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subset hb-subset related bugs tests
Projects
None yet
Development

No branches or pull requests

2 participants