Skip to content

Remove uniffi build_foreign_language_testcases tests#858

Merged
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
thunderbiscuit:test/remove-weird-tests
Sep 11, 2025
Merged

Remove uniffi build_foreign_language_testcases tests#858
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
thunderbiscuit:test/remove-weird-tests

Conversation

@thunderbiscuit
Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit commented Sep 11, 2025

I think we should remove this test (only the Swift one remains, the others have been commented out because they didn't pass for some reason we couldn't figure out... basically proving my point) for 2 reasons:

  1. The integration between the Rust library, uniffi, and the languages themselves is already tested/captured by the unit tests in the Android and Swift testing workflows. Adding these here with some weird workaround that's a black box we don't really understand nor own is not optimal. I assume their purpose is to allow quick testing of the integration directly in the Rust tests, but that makes it hard to work with, and in most instances you end up running cargo test --lib just to get around them (that's indeed why we have it set up that way in the justfile... to get around this test).
  2. Removing those makes the Rust unit test purely focused on Rust. They're easier to run, don't fail for some odd reason we don't understand, and focus on the Rust side of the API. The Android and Swift side is up to us to test correctly with the appropriate test suite in the specific libraries.

I'm not against bringing them back one day if someone is interested in going down that rabbit hole, figuring out why they are useful, report back, and open a decent PR that uses them and shows why it's a plus for this codebase. For now, none of this has been done, and so their value is a net negative IMO.

Copy link
Copy Markdown
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 35dcab1

👍

@thunderbiscuit thunderbiscuit merged commit 35dcab1 into bitcoindevkit:master Sep 11, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants