Fix android tests #136
Fix android tests #136
Conversation
They had been silently ignored on our CI. Test resources are duplicated in src/androidInstrumentedTest/assets and src/commonTest/resources for now until we find a way to share them.
This fixes `WARNING: D8: Unexpected error during rewriting of Kotlin metadata for class` when building the android library.
|
There are no functional changes, and build changes are limited to Android. I've checked that the Android library is valid. |
There was an off-by-one error in how we freed dynamically allocated memory in case of failure. This only happens in methods that take arrays of pubkeys/nonces/partial signatures, and only when they fail.
t-bast
left a comment
t-bast
left a comment
There was a problem hiding this comment.
LGTM, but I have no idea what side-effects those builds changes could have. I think that before merging this PR, we need to test it all the way to Phoenix to be completely safe?
Since ACINQ/bitcoin-kmp#169 also needs to be tested all the way to lightning-kmp and eclair, my recommendation would be:
- update ACINQ/bitcoin-kmp#169 to use this SNAPSHOT version of
secp256k1-kmpand test it locally - open a PR in
lightning-kmpthat uses the SNAPSHOT version oflightning-kmpfrom ACINQ/bitcoin-kmp#169 - open a PR in
bitcoin-libto prepare a release matching ACINQ/bitcoin-kmp#169 - open a PR in
eclairusing this SNAPSHOT version ofbitcoin-lib - ideally, if Dominique is up for it, create a version of Phoenix that uses the SNAPSHOT version of
lightning-kmpto verify that nothing weird happens?
At that point if everything works, we can merge all that chain of PRs and release everything. WDYT? Is this overkill?
Yes it makes sense to update ACINQ/bitcoin-kmp#169), I've opened relevant PRs. |
t-bast
left a comment
t-bast
left a comment
There was a problem hiding this comment.
LGTM, we can merge this and create a separate PR to set the version to 22.0 for a release 👍
I have broken them during a recent update to the build scripts and they had been silently ignored on our CI, which can be seen when running
gradlew connectedCheck --info, it will show:Skipping task ':jni:android:connectedCheck' as it has no actions.To verify that the problem has been fixed, start en emulator and run
gradlew connectedCheck, you will see:Test resources are duplicated in src/androidInstrumentedTest/assets and src/commonTest/resources for now until we find a way to share them.