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

[WIP] Better bindings tests #2348

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Dec 9, 2024

I'm sharing this one early, before I get too deep in it. I only implement the first few basic tests and only in Python, but hopefully it paints a fairly clear picture of what I'm going for. WDYT?


This comes from my experiences with #2333 and uniffi-bindgen-gecko-js. For those, I needed to run a lot of tests while (re)implementing a bindings generator. I love how the current tests cover basically all UniFFI features, but I think there a few things that can be improved. This commit adds a suite of tests specifically aimed at the bindings.

The new tests target specific features rather than trying to mimic real-world examples. Before, I spent a lot of time trying to figure out what a test failure meant. I'm hoping that when one of these tests fail, it's obvious what's not working. It also means that the new test suites don't require writing so much foreign code.

They also have a better external bindings story. I'm thinking we can direct external bindings authors to copy everything the bindings-tests/fixtures sub-directories and write their own version of bindings-tests/tests/tests.rs. They get to use the macro now, since it's doesn't hard code the mapping from file extension to test runner. Copying the code feels a bit weird, but I like it better than having to publish all these test fixture crates and I think it will work okay in practice.
I still want to keep the current examples/fixtures. They're nice because they showcase real-world use-cases and test that those use-cases work like we expect them to. If this gets merged, we can rework those crates to focus on that. Maybe we rework them to only test one bindings language per example/fixture.

@bendk bendk requested a review from a team as a code owner December 9, 2024 22:41
@bendk bendk requested review from jeddai and removed request for a team December 9, 2024 22:41
@bendk bendk force-pushed the push-moxsutuzotvp branch from a4f02d0 to cb2cb25 Compare December 9, 2024 23:34
@bendk bendk changed the title [WIP] Better tests [WIP] Better bindings tests Dec 9, 2024
@bendk bendk force-pushed the push-moxsutuzotvp branch from cb2cb25 to a85bcd0 Compare December 10, 2024 00:36
This comes from my experiences with mozilla#2333 and uniffi-bindgen-gecko-js.
For those, I needed to run a lot of tests while (re)implementing a
bindings generator.  I love how the current tests cover basically all
UniFFI features, but I think there a few things that can be improved.
This commit adds a suite of tests specifically aimed at the bindings.

The new tests target specific features rather than trying to mimic
real-world examples.  Before, I spent a lot of time trying to figure out
what a test failure meant.  I'm hoping that when one of these tests
fail, it's obvious what's not working.  It also means that the new test
suites don't require writing so much foreign code.

They also have a better external bindings story.  I'm thinking we can
direct external bindings authors to copy everything the
`bindings-tests/fixtures` sub-directories and write their own version of
`bindings-tests/tests/tests.rs`. They get to use the macro now, since
it's doesn't hard code the mapping from file extension to test runner.
Copying the code feels a bit weird, but I like it better than having to
publish all these test fixture crates and I think it will work okay in
practice.

I still want to keep the current examples/fixtures. They're nice because
they showcase real-world use-cases and test that those use-cases work
like we expect them to. If this gets merged, we can rework those crates
to focus on that.  Maybe we rework them to only test one bindings
language per example/fixture.
@bendk bendk force-pushed the push-moxsutuzotvp branch from a85bcd0 to 81e6fc6 Compare December 10, 2024 00:37
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.

1 participant