Allow top level payjoin-ffi testing with ./contrib.test.sh

[benalleng]

Because of how our dev shell puts us in the msrv and currently the testing infra in payjoin-ffi prevents the tests being run at the top level when we are in a dev shell it would be nice to fix/allow this behavior for more sweeping testing.

rust-payjoin/payjoin-ffi/contrib/test.sh

Lines 1 to 10 in 4186232

	#!/usr/bin/env bash
	set -e
	RUST_VERSION=$(rustc --version | awk '{print $2}')
	if [[ ! $RUST_VERSION =~ ^1\.85\. ]]; then
	cargo test --package payjoin-ffi --verbose --features=_manual-tls,_test-utils
	else
	echo "Skipping payjoin-ffi tests for Rust version $RUST_VERSION (MSRV)"
	fi

[caarloshenriq]

Hey, I'd like to work on this issue.

I tested running cargo test --package payjoin-ffi --verbose --features=_manual-tls,_test-utils directly on 1.85.0 and it passes without errors, so the skip doesn't appear necessary.

My proposed fix would be to simplify payjoin-ffi/contrib/test.sh by removing the version check entirely, so tests always run regardless of the Rust version.

Does that align with what you had in mind? Are there any other concerns I should be aware of before opening a PR?

[benalleng]

I think this just mostly seems like a noop at this point right? ideally we could have it like the contrib/test.sh at the workspace level where it crawls through the individual bindings and runs their tests

rust-payjoin/contrib/test.sh

Lines 46 to 51 in 5dcce43

	for crate in $CRATES; do
	(
	cd "$crate"
	./contrib/test.sh
	)
	done

In this case running ./contrib/test.sh would run all the tests at every level in the ffi when run from the repo root

I think I misspoke in my original comment, not quite sure what I meant there.

[spacebear21]

I think this just mostly seems like a noop at this point right? ideally we could have it like the contrib/test.sh at the workspace level where it crawls through the individual bindings and runs their tests

@Mshehu5 already did some pre-requisite work to facilitate this so should be really straightforward now.

My only concern with this approach is it would include downstream language bindings generation and testing into the base rust CI, and we really shouldn't entangle the testing the core rust library tests with testing downstream language. That seems likely to lead to developer confusion and frustration. We could maybe just remove payjoin-ffi from the root level contrib/test.sh?

[caarloshenriq]

We could maybe just remove payjoin-ffi from the root level contrib/test.sh?

So the fix would simply be removing payjoin-ffi from the CRATES list in the root contrib/test.sh? Like the exemple below:

CRATES="payjoin payjoin-cli payjoin-mailroom"

Payjoin-ffi would still be covered by its own CI, and the language bindings (dart, javascript, python) already have their own contrib/test.sh scripts and workflows as well, so nothing would go untested.

Is that the intended fix, or is there anything else that should be addressed here?

[spacebear21]

removing payjoin-ffi from the CRATES list in the root contrib/test.sh

this + updating the payjoin-ffi contrib/test.sh script to remove the version branching then loop over all language bindings and run their respective contrib/test.sh.