Skip to content

feat(auth): Add auth emulator support#523

Merged
ssbushi merged 13 commits intomasterfrom
add-auth-emulator-support
Apr 16, 2021
Merged

feat(auth): Add auth emulator support#523
ssbushi merged 13 commits intomasterfrom
add-auth-emulator-support

Conversation

@ssbushi
Copy link
Copy Markdown
Contributor

@ssbushi ssbushi commented Apr 8, 2021

Discussion

Add support to communicate with the Auth Emulator #493.

Testing

  • Unit tests.
  • Manual testing.

API Changes

  • N/A.

@ssbushi ssbushi marked this pull request as ready for review April 8, 2021 17:56
@ssbushi ssbushi requested review from hiranya911 and yuchenshi April 8, 2021 17:56
Copy link
Copy Markdown
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ssbushi. Looks mostly good. My main observations are:

  1. We should avoid duplication by adding emulator support to the existing FirebaseTokenVerfierImpl class, as opposed to implementing a whole new class.
  2. Avoid duplication by putting env variable lookups in a shared util.
  3. We need a better way to test the functionality of all APIs in emulator mode. In other languages we used techniques like test parameterization and fixtures to achieve this. We should try to find an equivalent strategy for junit4 (although it's ok to do so in a separate PR).

@ssbushi ssbushi requested a review from hiranya911 April 14, 2021 22:47
Copy link
Copy Markdown
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes. Looks pretty good. Just a few more minor updates needed.

@ssbushi ssbushi requested a review from hiranya911 April 15, 2021 15:32
Copy link
Copy Markdown
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@hiranya911 hiranya911 changed the title Add auth emulator support feat(auth): Add auth emulator support Apr 15, 2021
@ssbushi ssbushi merged commit 1e26ef3 into master Apr 16, 2021
@ssbushi ssbushi deleted the add-auth-emulator-support branch April 16, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants