Upgrade to Kotlin 1.9.20 & gradle 8

[Daeda88]

This updates the gradle build tools to Gradle 8, moves to Kotlin 1.8.21, coroutines 1.7.1 and cleans up the build stack

• Moves build to Gradle 8
• Upgrade kotlin versions to 1.8.21, coroutines 1.7.1, serialization to 1.5.1
• Upgrades firebase to 32..0.0 (android) and 10.9.0 (iOS)
• Use Android Source Layout V2
• Use version variables in gradle.properties to ease future upgrades

[Daeda88]

Once this is merged, I can make a few more PRs from our Splendo fork, including fixes to Polymorphic serialization (it fails for nesting, doesnt support non-sealed polymorphism etc), several iOS methods, and tracking the new firestore persistence settings

[nbransby]

Another huge PR 🙄

What are the podspecs for?

[Daeda88]

Another huge PR 🙄

What are the podspecs for?

To get them to build. Im guessing the latest plugin requires them (which is a bit weird since we set noPodspec which implies you shouldnt need them). But this is on the native plugin side.

And yeah, its kind of big because the new Android filestructure requires renames from androidAndroidTest to androidInstrumentedTest etc. But the old way will get deprecated so you'll have to do the renames eventually.

[nbransby]

And yeah, its kind of big because the new Android filestructure requires renames from androidAndroidTest to androidInstrumentedTest etc. But the old way will get deprecated so you'll have to do the renames eventually.

This PR includes upgrading kotlin.mpp.androidSourceSetLayoutVersion to 2? That's great as it was on our todo list but it would be super helpful if that was in the PR title of description somewhere. In fact this task alone would make a sizable PR and should really be submitted separately, that would really help us improve our TTM (time to merge) on this project 🙏

[Daeda88]

@nbransby I dont think it would reduce the size that much. Upgrading to Gradle 8 means that Android now needs the namespace set. As a result, the cleanup requires most Manifest files to be deleted. With 10 modules, that just adds up. Most of the files changed dont contain that much, except for the Build.gradle files which will always come in bulk.

I did try and split it up for you, but it wont reduce the PR size by much and is a lot of work on my side.

I have updated the initial PR comment to better reflect what changed btw

[Daeda88]

Alternatively you could merge #386 first, and Ill make sure I track the changes

[nbransby]

gradle check fails on #386, does it work on this branch?

[Daeda88]

@nbransby there was a merge issue with test-utils. I fixed them, so I expect CI to work now (tests run locally at least)

[nbransby]

does gradle check pass?

[Daeda88]

It does now. There's a bit of a bug/missing feature with Kotlin 1.8 where AndroidUnitTest automatically has a dependency on commonTest. This is problematic because the tests in this library are instrumented tests. Kotlin 1.9 allows the disabling of the dependency, but for now, I solved it by making an Annotation @IgnoreForAndroidUnitTest which will make sure the tests are not done for Android Unit tests specifically.

[nbransby]

do you have a link to the docs on Kotlin 1.9 allows the disabling of the dependency?

[Daeda88]

https://slack-chats.kotlinlang.org/t/8582216/is-it-expected-that-new-androidinstrumentedtest-sourceset-do#481668b4-9d5f-4ef4-b4c4-1fdd05769b37

Link to actual Slack message (includes the images) https://kotlinlang.slack.com/archives/C3PQML5NU/p1682443478051419?thread_ts=1675016886.869689&cid=C3PQML5NU

[nbransby]

I noticed gradle check doesn't run the android instrumented tests and gradle connectedCheck appears to hang...

> Task :firebase-firestore:connectedDebugAndroidTest
Starting 36 tests on Pixel 4 - 13

Any ideas on that?

[Daeda88]

@nbransby sorry I completely forgot to respond at the time. I had to upgrade our tooling to Kotlin 1.9.10 due to build issues with XCode 15 on older kotlin versions, so Ive amended the PR with the latest changes from master.

Re the connected devices, looks like the expected emulator was not there, Ill try and have a look. I can run connectedCheck just fine on my local machine, so its probably some CI configuration.

[Qw4z1]

Once this is merged, I can make a few more PRs from our Splendo fork, including fixes to Polymorphic serialization (it fails for nesting, doesnt support non-sealed polymorphism etc), several iOS methods, and tracking the new firestore persistence settings

@Daeda88, would any of the fixes you mention include a fix for #420?

[Qw4z1]

And yeah, its kind of big because the new Android filestructure requires renames from androidAndroidTest to androidInstrumentedTest etc. But the old way will get deprecated so you'll have to do the renames eventually.

This PR includes upgrading kotlin.mpp.androidSourceSetLayoutVersion to 2? That's great as it was on our todo list but it would be super helpful if that was in the PR title of description somewhere. In fact this task alone would make a sizable PR and should really be submitted separately, that would really help us improve our TTM (time to merge) on this project 🙏

@nbransby, is this todo list that you mention publicly available somewhere? If not, I think it could make it easier for people who wants to contribute to know where they should start. 😇

[nbransby]

@nbransby looks like CI is finally passing!

@Daeda88 I'm guessing the ios tests defeated you in the end 😅

[Daeda88]

JS tests actually. They fail randomly and I don't know why. Rerunning ci a bunch of times will eventually pass it.

[nbransby]

@nbransby, is this todo list that you mention publicly available somewhere? If not, I think it could make it easier for people who wants to contribute to know where they should start. 😇

@Qw4z1 yes plenty of places to start here https://github.com/GitLiveApp/firebase-kotlin-sdk/issues

[nbransby]

JS tests actually. They fail randomly and I don't know why. Rerunning ci a bunch of times will eventually pass it.

The error is

> Failed to execute all tests:
  :firebase-firestore:jsBrowserTest: java.lang.IllegalStateException: command '/Users/runner/.gradle/nodejs/node-v18.12.1-darwin-x64/bin/node' exited with errors (exit code: 1)

but if you look at the js test report html it says all the tests passed 😑

[Daeda88]

I'm currently upgrading our internal fork to 1.9.20 and there JS seems to pass fine. Now it's the Android tests XD. Once its stable I'll update this pr again.

[Daeda88]

@nbransby upgraded to Kotlin 1.9.20. On our fork this now builds reliable. HOWEVER the Android tests fail on MacOS latest: the auth api is giving some timeouts for the Android emulator. We've resolved it by running on a 4 core ubuntu machine, but that is not free (0,016 USD per minute, at a build time of approximately 7 min). If you know of a way to optimize id be happy to do it, but so far no luck.

[nbransby]

Have you got a link to the failed runs?

Are you still running the iOS tests on Mac?

[Daeda88]

Here's an example: https://github.com/splendo/firebase-kotlin-sdk/actions/runs/6903223212/job/18781490591
Still running iOS tests, but Ive split Android/iOS/JS over three jobs/machines to increase build time (and make tests better, since it ensures a clean firebase emulator)

[MattSkala]

@nbransby upgraded to Kotlin 1.9.20. On our fork this now builds reliable. HOWEVER the Android tests fail on MacOS latest: the auth api is giving some timeouts for the Android emulator. We've resolved it by running on a 4 core ubuntu machine, but that is not free (0,016 USD per minute, at a build time of approximately 7 min). If you know of a way to optimize id be happy to do it, but so far no luck.

I've tried running the Android tests on the standard ubuntu-latest runner and it seems to work fine: https://github.com/MattSkala/firebase-kotlin-sdk/actions/runs/6989496153/job/19018094669?pr=2

[nbransby]

Still running iOS tests, but Ive split Android/iOS/JS over three jobs/machines to increase build time (and make tests better, since it ensures a clean firebase emulator)

@Daeda88 are we able to do this over free machines with the android tests running on standard ubuntu-latest runner which according to @MattSkala seems to work

[Daeda88]

@nbransby @MattSkala Set it to Ubuntu latest and also upgraded to 1.9.21

[MattSkala]

@nbransby @MattSkala Set it to Ubuntu latest and also upgraded to 1.9.21

@Daeda88 I see the workflow failed, it would probably be better to get macOS working after all, can you give it one more shot?

After looking at the workflows, I think there might be a race condition as there is no wait until Firebase Emulator is started. It might happen that tests are started before the emulators are ready. We could wait for the emulator to start in the setup test action e.g. using wait-on: MattSkala@49dc48b

[Daeda88]

@MattSkala pushed MacOS 13 again. Doubt it works and I dont really see a good way to counter the race condition. Any suggestions?

[Daeda88]

@nbransby well waddayouknow, apparently it works fine on MacOS 13 for Kotlin 1.9.21. Merging is obviously blocked due to review & an expected build on api 29 instead of 34 which is latest, but that's nothing I can control :)