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

Support for Kotlin 2.0 #529

Merged
merged 32 commits into from
Jun 25, 2024
Merged

Conversation

Daeda88
Copy link
Contributor

@Daeda88 Daeda88 commented Jun 11, 2024

Changes required for Kotlin 2.0 / upgrades to latest firebase sdks

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 11, 2024

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

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

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 23, 2024

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

@Reedyuk
Copy link
Collaborator

Reedyuk commented Jun 23, 2024

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

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 24, 2024

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

Daeda88 commented Jun 24, 2024

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

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

Daeda88 commented Jun 24, 2024

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

Reedyuk commented Jun 24, 2024

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

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 24, 2024

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

Daeda88 commented Jun 24, 2024

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

Daeda88 commented Jun 24, 2024

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

Daeda88 commented Jun 25, 2024

Fixed issue. Was caused by calling runBlockingTest instead of runTest

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 25, 2024

@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

@nbransby nbransby enabled auto-merge June 25, 2024 11:47
auto-merge was automatically disabled June 25, 2024 12:35

Head branch was pushed to by a user without write access

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 25, 2024

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

@Daeda88
Copy link
Contributor Author

Daeda88 commented Jun 25, 2024

iOS failed due to some cocoapods issue, Restarting should work

@Daeda88 Daeda88 requested a review from nbransby June 25, 2024 15:26
@nbransby nbransby merged commit a0599d7 into GitLiveApp:master Jun 25, 2024
14 checks passed
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.

4 participants