Storage Emulator support in rules-unit-testing #4863
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 9a885a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Changeset File Check
|
Binary Size ReportAffected SDKs
Test Logs |
|
Very nice 😍 |
abeisgoat
approved these changes
May 4, 2021
samtstern
commented
May 6, 2021
| }; | ||
| /** Construct an App authenticated as an admin user. */ | ||
| export function initializeAdminApp(options: AdminAppOptions): firebase.app.App { | ||
| export function initializeAdminApp(options: AdminAppOptions): app.App { |
There was a problem hiding this comment.
This type has been wrong for a while! We can't live with it anymore because of Storage, which has a totally different API between client and Admin.
samtstern
commented
May 6, 2021
| ssl: false | ||
| }); | ||
| const { hostname, port } = parseHost(getFirestoreHost()); | ||
| app.firestore().useEmulator(hostname, port); |
There was a problem hiding this comment.
Moving to consistently use useEmulator across the board.
samtstern
commented
May 6, 2021
| @@ -216,7 +220,24 @@ describe('Testing Module Tests', function () { | |||
| }); | |||
| }); | |||
|
|
|||
| it('initializeAdminApp() has admin access', async function () { | |||
| it('initializeAdminApp() has admin access to RTDB', async function () { | |||
There was a problem hiding this comment.
Separated this into three tests so we can tell failures apart.
samtstern
commented
May 6, 2021
| @@ -15,7 +15,7 @@ | |||
| "build:deps": "lerna run --scope @firebase/rules-unit-testing --include-dependencies build", | |||
| "dev": "rollup -c -w", | |||
| "test:nyc": "TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --config ../../config/mocharc.node.js", | |||
| "test": "firebase --project=foo --debug emulators:exec 'yarn test:nyc'", | |||
| "test": "FIREBASE_CLI_PREVIEWS=storageemulator STORAGE_EMULATOR_HOST=http://localhost:9199 firebase --project=foo --debug emulators:exec 'yarn test:nyc'", | |||
There was a problem hiding this comment.
I had to add the STORAGE_EMULATOR_HOST variable to compensate for firebase/firebase-tools#3337
There was a problem hiding this comment.
Suggested change
| "test": "FIREBASE_CLI_PREVIEWS=storageemulator STORAGE_EMULATOR_HOST=http://localhost:9199 firebase --project=foo --debug emulators:exec 'yarn test:nyc'", | |
| "test": "FIREBASE_CLI_PREVIEWS=storageemulator firebase --project=foo --debug emulators:exec 'yarn test:nyc'", |
Gotta clean this up now we've released the CLI with the fix
samtstern
requested review from
avolkovi,
Feiyang1,
hiranya911,
hsubox76 and
sam-gc
as code owners
Size Analysis ReportAffected Products
|
yuchenshi
approved these changes
May 6, 2021
hsubox76
approved these changes
May 6, 2021
sam-gc
approved these changes
May 6, 2021
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discussion
Adds support for Storage emulator to the rules-unit-testing library.
TODO:
Nice to have:
Testing
See unit tests. Currently we have the following failures, so we have to skip a test in CI:
(1) is a known issue with Storage not being able to work without credentials even when using the emulator. @abeisgoat or @hiranya911 do you know of any way to fix this without adding service account credentials to the JS SDK test environment?
API Changes
initializeAdminApp()has always been returning afirebase.app.Apptype from the client SDK but that has not been correct for a few releases! So I did change the type which is an API change, but it's to fix a bug.