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

Upgrade to sqldelight 2 #546

Merged
merged 4 commits into from
May 17, 2024
Merged

Upgrade to sqldelight 2 #546

merged 4 commits into from
May 17, 2024

Conversation

dpad85
Copy link
Member

@dpad85 dpad85 commented Apr 3, 2024

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 (#112).

@dpad85 dpad85 requested a review from robbiehanson April 3, 2024 16:29
@robbiehanson
Copy link
Contributor

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

@dpad85
Copy link
Member Author

dpad85 commented Apr 4, 2024

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
Copy link
Member Author

dpad85 commented Apr 4, 2024

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

@robbiehanson
Copy link
Contributor

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
Copy link
Contributor

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.

@dpad85
Copy link
Member Author

dpad85 commented May 17, 2024

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
Copy link
Contributor

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?)

@dpad85 dpad85 merged commit d871078 into master May 17, 2024
@dpad85 dpad85 deleted the sqldelight2 branch May 17, 2024 17:00
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.

2 participants