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

Remove goldens for platform dependent tests #278

Closed
liamappelbe opened this issue May 11, 2022 · 1 comment · Fixed by dart-archive/ffigen#363
Closed

Remove goldens for platform dependent tests #278

liamappelbe opened this issue May 11, 2022 · 1 comment · Fixed by dart-archive/ffigen#363
Assignees
Labels
lang-objective_c Related to Objective C support package:ffigen

Comments

@liamappelbe
Copy link
Contributor

Our ObjC tests that verify that the output matches a golden can only be regenerated on mac devices. This makes it hard to contribute from non-mac devices. These tests aren't super necessary anymore either, since we have good integration test coverage of our ObjC features, in native_objc_test. So we should probably just remove them.

I'm not sure that all the C features are fully covered by integration tests, so we should keep those goldens. I'm only proposing removing the ObjC ones, because those are the ones that are platform dependent.

@dcharkes
Copy link
Collaborator

I'm not sure that all the C features are fully covered by integration tests, so we should keep those goldens.

Integration tests do not cover all ABIs. For example we run the tests only on 64 bit platforms, so if we emit the wrong integer type that happens to be correct on all 64 bit platforms we would miss that.

I can see how having the ObjectiveC tests only being able to run on MacOS is problematic. So, having no goldens for ObjectiveC but keeping them for C sounds like a good compromise to me. 👍

cc @mannprerak2.

@liamappelbe liamappelbe transferred this issue from dart-archive/ffigen Nov 15, 2023
parlough pushed a commit to parlough/native that referenced this issue Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-objective_c Related to Objective C support package:ffigen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants