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.

Sign up for GitHub

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

Jump to bottom

Re-implement Unified Emulator Settings #1802

Merged
merged 21 commits into from
Jul 27, 2020
Merged

Re-implement Unified Emulator Settings #1802

merged 21 commits into from
Jul 27, 2020

Conversation

samtstern
Copy link
Contributor

@samtstern samtstern commented Jul 22, 2020 •
edited
Loading

History:
#1672 #1688 #1690 #1712 #1737

This follows Proposal 2 at:
http://go/firebase-emulator-connection-api

TODO:

  • Regenerate api.txt
  • Version bumps

@googlebot googlebot added the cla: yes Override cla label Jul 22, 2020
@google-oss-bot

This comment has been minimized.

Sign in to view
@google-oss-bot

This comment has been minimized.

Sign in to view
@samtstern samtstern mentioned this pull request Jul 22, 2020
Closed
1 task
@samtstern
Copy link
Contributor Author

samtstern commented Jul 22, 2020 •
edited
Loading

Not sure what's going on with the tests, every test I have run locally passes (connected and unit)

Nevermind, looks like firebase-messaging:

> Task :firebase-messaging:testReleaseUnitTest

com.google.firebase.messaging.FirebaseMessagingRoboTest > testUnsubscribeFromTopic_withPrefix FAILED
    java.lang.Exception
        Caused by: com.google.common.truth.AssertionErrorWithFacts at FirebaseMessagingRoboTest.java:113

@samtstern
Copy link
Contributor Author

/test device-check-changed

@VinayGuthal
Copy link
Contributor

/test api-information

@VinayGuthal
Copy link
Contributor

/test check-changed

@samtstern samtstern requested a review from vkryachko July 23, 2020 14:47
@samtstern samtstern requested a review from vkryachko July 23, 2020 15:42
@samtstern
Copy link
Contributor Author

/retest

@samtstern
Copy link
Contributor Author

/test device-check-changed

@samtstern
Copy link
Contributor Author

@schmidt-sebastian @vkryachko all review comments addressed, TODOs done, tests are now passing.

@vkryachko
Copy link
Member

looks like at least one of the failures is legit:

com.google.firebase.firestore.FirebaseFirestoreTest > getInstance_withEmulator_mergeSettingsFailure FAILED
    java.lang.AssertionError at FirebaseFirestoreTest.java:72

@samtstern
Copy link
Contributor Author

@vkryachko thanks you're right we no longer throw there.

@samtstern
Copy link
Contributor Author

/test device-check-changed

@samtstern samtstern merged commit 57ffa1f into master Jul 27, 2020
@firebase firebase locked and limited conversation to collaborators Aug 27, 2020
@kaibolay kaibolay deleted the ss-emulator-settings-redo branch September 14, 2022 17:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@schmidt-sebastian schmidt-sebastian schmidt-sebastian approved these changes

@vkryachko vkryachko Awaiting requested review from vkryachko

Assignees
No one assigned
Labels
cla: yes Override cla size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@samtstern @google-oss-bot @VinayGuthal @vkryachko @schmidt-sebastian @googlebot