GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
1.11k stars 153 forks source link

Support for Kotlin 2.0 #529

Closed Daeda88 closed 3 months ago

Daeda88 commented 3 months ago

Changes required for Kotlin 2.0 / upgrades to latest firebase sdks

Daeda88 commented 3 months ago

As part of this I've also tried to get the JVM tests to work. Most of them are running now, except for ones in performance and auth. Im leaving that open for discussion in this ticket. Either the functionality should be brought to the JVM sdk or the tests should be ignored.

nbransby commented 3 months ago

I think remote config tests will need to be skipped for jvm too for now

Daeda88 commented 3 months ago

Yep, added and disabled for: Analytics, Config, Performance and Storage. I'll leave actually fixing that to the GitLive team :)

Reedyuk commented 3 months ago

Hmm seems to be getting stuck on the ci when running the ios tests, ive not had chance to run this locally yet.

Daeda88 commented 3 months ago

I upgraded the pods / updated the OS, so ci can't use the cached pods. Downloading them takes a LOOOONG time on CI, because they are huge. Runs fine for me locally in any case

EDIT: I am getting hanging tests on ios storage, but Im getting that on master as well. Also based on the logs its just not compiling firestore so most likely this is unrelated

Daeda88 commented 3 months ago

So issue is somewhere in the storage module. Not sure what broke there except maybe some regression on the cocoapods. But disabling those tests for iOS gets the check to green

siarhei-luskanau commented 3 months ago

@Reedyuk @Daeda88 Let we split the JVM tests and iOS tests by modules like android tests on the CI. In this case we reduce the Github Actions time, but we will consume more build agents in parallel. Additional we will see issue in specific modules.

Daeda88 commented 3 months ago

I can reproduce it locally. Firebase Storage does no launch on iOS. I think it may be related to using the SimulatorArm64 tests instead of the X64 one, but unsure. Even observing the upload task does not fire, so something is wrong there.

Reedyuk commented 3 months ago

for now, could we not use macos-12 or macos-13? i beleive they are intel based

Daeda88 commented 3 months ago

Trying that atm, but also seeing slow build times there. Ive also quickly tried to write a pure iOS app to see if I could reproduce the issue there, but it seems to work fine. All in all, no idea what's causing the failure yet.

Daeda88 commented 3 months ago

@Reedyuk @nbransby I have reason to suspect this may have been broken for a while now. I tried checking the master and locally I get the same issues. So I started looking at past CI actions and I suspect these tests may have been skipped for some reason. For instance, see the last merge to master: https://github.com/GitLiveApp/firebase-kotlin-sdk/actions/runs/9632168268/job/26564964714#step:5:258

Or the PR where storage was recently changed: https://github.com/GitLiveApp/firebase-kotlin-sdk/actions/runs/9646466575/job/26602942910?pr=529#step:5:259

No idea why it was skipped in those branches, but it looks like This is due to Github actions being set to macos latest, which nowadays is MacOS 14 with an M2 chip, so X64 tests wont run. I therefor conclude that these tests are simply not working and it has nothing to do with the K2 branch

EDIT: final proof these never worked https://github.com/GitLiveApp/firebase-kotlin-sdk/actions/runs/9515996285/job/26231653421#step:5:237

Daeda88 commented 3 months ago

Made issue #547 for further discussion as this should be out of scope for this PR. For now Ive disabled the storage tests on iOS

Daeda88 commented 3 months ago

Fixed issue. Was caused by calling runBlockingTest instead of runTest

Daeda88 commented 3 months ago

@Reedyuk @nbransby with the exception of Crashlytics test, which remain a bit unstable due to how they work, all green now. Also fixed issues with storage on JS so its now on par with the other platforms

Daeda88 commented 3 months ago

@nbransby can you reenabled auto merge. The ios tests got broken by the latest master. Fixed now

Daeda88 commented 3 months ago

iOS failed due to some cocoapods issue, Restarting should work