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

Emulator fixes for #1281, #1279, and #1277 #1287

Merged
merged 14 commits into from
May 14, 2019
Merged

Conversation

abeisgoat
Copy link
Contributor

In this I've done some work to clean up some of our tests and I've refactored FunctionsEmulator to use more static methods so it's easier to stub out calls for testing.

We also see an introduction of tests for the core emulator "hub" which routes the requests.

@googlebot googlebot added the cla: yes Manual indication that this has passed CLA. label May 13, 2019
@coveralls
Copy link

coveralls commented May 13, 2019

Coverage Status

Coverage increased (+1.7%) to 62.483% when pulling 4793acf on ah-emulator-patches into 7251c1f on master.

Copy link
Contributor

@samtstern samtstern left a comment

Choose a reason for hiding this comment

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

Small comments, mostly looks great and love the addtl test coverage.

externalRoute,
`${externalRoute}/*`,
];
const httpsFunctionRoutes = [httpsFunctionRoute, `${httpsFunctionRoute}/*`];
Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to make sure that callable functions get this treatment, not just "normal" HTTPS functions. Can you confirm (or maybe even add a test)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point good point, should add a test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed callable still work, will look at adding a test, but prolly not for this PR

src/emulator/functionsEmulator.ts Show resolved Hide resolved
src/emulator/functionsEmulator.ts Outdated Show resolved Hide resolved
src/emulator/functionsEmulator.ts Outdated Show resolved Hide resolved
src/emulator/functionsEmulator.ts Show resolved Hide resolved
src/emulator/functionsEmulator.ts Show resolved Hide resolved
src/test/emulators/fixtures.ts Show resolved Hide resolved
src/test/emulators/functionsEmulator.spec.ts Outdated Show resolved Hide resolved
@samtstern
Copy link
Contributor

@abeisgoat

ERROR: /home/travis/build/firebase/firebase-tools/src/test/emulators/functionsEmulator.spec.ts:10:14 - require statement not part of an import statement

@abeisgoat
Copy link
Contributor Author

Bleh, let's try again then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Manual indication that this has passed CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants