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

Fix tests #13

Merged
merged 3 commits into from
May 7, 2024
Merged

Fix tests #13

merged 3 commits into from
May 7, 2024

Conversation

fmasa
Copy link
Contributor

@fmasa fmasa commented Apr 22, 2024

PR tests were previously failing because there are currently two tests that both initialize the default Firebase app.

I originally wanted to just use different instance per each test, but FirebaseApp internals always expect that default app exist, so I added a simple cleanup before each test instead.

@fmasa fmasa marked this pull request as draft April 22, 2024 18:46
@fmasa
Copy link
Contributor Author

fmasa commented Apr 23, 2024

@nbransby The scacap/action-surefire-report GH action does not play well with PRs opened from forks. Should I try to fix it, or do we drop it altogether? 🤔

@nbransby
Copy link
Member

Are we using something else for the firebase-kotlin-sdk?

@fmasa
Copy link
Contributor Author

fmasa commented Apr 23, 2024

@nbransby For firebase-kotlin-sdk the test output is only attached to a Job when test failed..

But that is only for other targets. There is no CI running for JVM builds in firebase-kotlin-sdk 🤔

@nbransby
Copy link
Member

@nbransby For firebase-kotlin-sdk the test output is only attached to a Job when test failed..

we can do the same here if thats easiest

But that is only for other targets. There is no CI running for JVM builds in firebase-kotlin-sdk 🤔

Yeah we probably should add that sometime!

@fmasa fmasa changed the title Destroy all Firebase instances before each tests Fix tests May 7, 2024
@fmasa
Copy link
Contributor Author

fmasa commented May 7, 2024

@nbransby I dropped the problematic action. Didn't get to this sooner. 🙂

@fmasa fmasa marked this pull request as ready for review May 7, 2024 17:17
@nbransby nbransby merged commit 1a779c1 into GitLiveApp:master May 7, 2024
1 check passed
@fmasa fmasa deleted the fix-tests branch May 7, 2024 21:32
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