ACINQ / phoenix

Phoenix is a self-custodial Bitcoin wallet using Lightning to send/receive payments.
https://phoenix.acinq.co
Apache License 2.0
654 stars 97 forks source link

Upgrade to sqldelight 2 #546

Closed dpad85 closed 4 months ago

dpad85 commented 6 months ago

This PR upgrades Phoenix to SQLDelight v2.0.1 (see changes: https://github.com/cashapp/sqldelight/releases/tag/2.0.0)

Will need tests to check reproducibility issues are fixed (https://github.com/ACINQ/phoenix/issues/112).

robbiehanson commented 6 months ago

I can't get ./gradlew iosX64Test to work on my machine... Are you seeing similar issues on macOS ?

dpad85 commented 6 months ago

iosX64Test builds successfully and I'm also able to run the phoenix-shared tests from the IDE for iosX64. Tests all pass, reportedly.

However there are errors reported in the logs, for example in SqlitePaymentsDatabaseTest.ougoing__do_not_reuse_ids. I'm not sure if that's because this particular test does assertFails that leads to printerr (because of sql constraint violations, which is what we're testing), or if there's actually a deeper issue.

Running the same test on Android also passes, but it does not print errors.

dpad85 commented 6 months ago

Can you try again using latest commit ? (9aae403)

robbiehanson commented 6 months ago

I'm getting this:

> Task :phoenix-shared:iosX64Test
    at 22  test.kexe                           0x102a1fc57        kfun:app.cash.sqldelight.db.SqlDriver#execute(kotlin.Int?;kotlin.String;kotlin.Int;kotlin.Function1<app.cash.sqldelight.db.SqlPreparedStatement,kotlin.Unit>?){}app.cash.sqldelight.db.QueryResult<kotlin.Long>-trampoline + 103 (/Users/runner/work/sqldelight/sqldelight/runtime/src/commonMain/kotlin/app/cash/sqldelight/db/SqlDriver.kt:63:3)
    at 45  test.kexe                           0x102863d9d        kfun:kotlin.coroutines.native.internal.BaseContinuationImpl#resumeWith(kotlin.Result<kotlin.Any?>){} + 829 (/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/coroutines/ContinuationImpl.kt:30:39)
    at 46  test.kexe                           0x1029a17e9        kfun:kotlin.coroutines.Continuation#resumeWith(kotlin.Result<1:0>){}-trampoline + 73 (/opt/buildAgent/work/2fed3917837e7e79/kotlin/libraries/stdlib/src/kotlin/coroutines/Continuation.kt:26:12)
    at 47  test.kexe                           0x102aa7018        kfun:kotlinx.coroutines.DispatchedTask#run(){} + 2728 (/opt/buildAgent/work/44ec6e850d5c63f0/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt:108:71)

> Task :phoenix-shared:iosX64Test FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':phoenix-shared:iosX64Test'.
> Multiple build operations failed.
      Could not write XML test results for fr.acinq.phoenix.utils.CsvWriterTests to file /Users/robbie/Programs/Acinq/phoenix/phoenix-shared/build/test-results/iosX64Test/TEST-fr.acinq.phoenix.utils.CsvWriterTests.xml.
      Could not write XML test results for fr.acinq.phoenix.db.SqlitePaymentsDatabaseTest to file /Users/robbie/Programs/Acinq/phoenix/phoenix-shared/build/test-results/iosX64Test/TEST-fr.acinq.phoenix.db.SqlitePaymentsDatabaseTest.xml.
      Could not write XML test results for fr.acinq.phoenix.db.serializers.OutpointDbDbSerializerTest to file /Users/robbie/Programs/Acinq/phoenix/phoenix-shared/build/test-results/iosX64Test/TEST-fr.acinq.phoenix.db.serializers.OutpointDbDbSerializerTest.xml.

with many more messages of: "Could not write XML test results for ..."

robbiehanson commented 5 months ago

Investigating this further, it might be some kind of weird incompatibility with SKIE.

That is, if I use commit e84b6aa54, I can run iosX64Test successfully. If I use the next commit ad64af39f ("(ios) Integrate Touchlab's SKIE (#529)"), then I get an error.

Following on this idea, I tested rebasing this branch on master (now that SKIE has been reverted), and it's working fine for me.

dpad85 commented 4 months ago

Investigating this further, it might be some kind of weird incompatibility with SKIE.

That is, if I use commit e84b6aa, I can run iosX64Test successfully. If I use the next commit ad64af3 ("(ios) Integrate Touchlab's SKIE (#529)"), then I get an error.

Following on this idea, I tested rebasing this branch on master (now that SKIE has been reverted), and it's working fine for me.

@robbiehanson I rebased this branch on master (so it includes the commit removing SKIE). Initial tests on iOS look good.

I had to add af9d919 to fix a compilation issue, can you take a look and make sure this is correct?

robbiehanson commented 4 months ago

I ran iosX64Test, and it worked well on my machine.

Well, the first time I ran it I received errors like above. Then I restarted my machine, and it worked fine. So I suspect there may be issues with gradle (cache?)