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

Export Server #463

Closed
wants to merge 1 commit into from
Closed

Export Server #463

wants to merge 1 commit into from

Conversation

penx
Copy link

@penx penx commented Jun 5, 2022

Add export * from server to index.ts.

Use case

I would like to use @google-cloud/functions-framework for integration tests of a Remix Firebase functions adapter, to start and stop a functions server during Playwright tests.

The existing integration tests for Remix use their Express adapter:

      let port = await getPort();
      let app = express();
      app.use(express.static(path.join(fixture.projectDir, "public")));
      app.all(
        "*",
        createExpressHandler({ build: fixture.build, mode: "production" })
      );

      let server = app.listen(port);

https://github.com/remix-run/remix/blob/6a2bf52d9ca60e2ce6459ebe72df28e6a568ed26/integration/helpers/create-fixture.ts#L97-L121

I would like to run the same tests against a Firebase adapter I am working on.

The difference between the Express adapter and Firebase adapter is primarily the use of rawBody, and I'd like to ensure the integration tests still succeed given this difference. I need a functions server to use in the integration tests that behaves as per production deployments of Firebase functions.

I can update the above code to the following:

      let port = await getPort();
      let functionsPort = await getPort();
      let app = express();
      app.use(express.static(path.join(fixture.projectDir, "public")));
      app.all("*", proxy(`localhost:${functionsPort}`));
      let server = app.listen(port);
      let functionsServer = functions.getServer(
        createFirebaseHandler({
          build: fixture.build,
          mode: "production",
        }),
        "http"
      );
      functionsServer.listen(functionsPort);

...but this requires access to the getServer function.

I looked at other libraries such as the emulators in firebase-tools, but these require the functions to exist as files on disk. In the tests above, the functions need to be created per test using fixture.build as an argument.

The getTestServer exported from testing.ts has 2 issues preventing me from using it for this purpose:

  • TypeScript cannot find the types, and errors with "Cannot find module '@google-cloud/functions-framework/testing' or its corresponding type declarations.ts(2307)"
  • It assumes the function exists as a file, and accepts a function name as a string, but as shown in my example code above I need to build and pass the function directly.

In order to support using `@google-cloud/functions-framework` in integration tests, add `./server` to exports
@penx
Copy link
Author

penx commented Jun 6, 2022

Since opening this, I've realised that I can use getTestServer as follows:

import { http } from "@google-cloud/functions-framework";
import { getTestServer } from "@google-cloud/functions-framework/testing";

http(
  "remixServer",
  createFirebaseHandler({
    build: fixture.build,
    mode: "production",
  })
);
let functionsServer = getTestServer("remixServer");

However:

  • this isn't in the testing.md docs, perhaps worth adding?
  • there are typescript issues when doing import { getTestServer } from "@google-cloud/functions-framework/testing" (Cannot find module '@google-cloud/functions-framework/testing' or its corresponding type declarations.ts(2307)), which may be resolved by fix: TypeScript exports in package.json #461
  • I expect there are other use cases where having access to getServer would be useful

Copy link
Contributor

@grant grant 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 the PR. However, this is intentional.

Can you file a GitHub issue first before a PR?

@@ -16,6 +16,7 @@
* @public
*/
export * from './functions';
export * from './server';
Copy link
Contributor

Choose a reason for hiding this comment

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

This lack of re-export is intentional.

The usage should be:

import {getTestServer} from '@google-cloud/functions-framework/testing';

Or, if you are trying to get server, you can import from /server.

See docs:

https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/master/docs/testing-functions.md

Copy link
Author

Choose a reason for hiding this comment

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

Or, if you are trying to get server, you can import from /server.

Thanks, though server isn’t currently included in the package.json exports

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Why do you want to import the internal server vs the test server?

Perhaps a different approach can be taken, since we can't export our internal server.

We have examples of integration tests.

Copy link
Author

@penx penx Jun 6, 2022

Choose a reason for hiding this comment

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

See my follow up comment here.

#463 (comment)

I don’t think I need this anymore as I was able to figure out how to load a function in code.

I think the docs could be updated to show how you can test a function without it existing on disk.

@penx penx closed this Jun 6, 2022
@grant grant self-assigned this Jun 6, 2022
@ChrisChiasson
Copy link

For those still getting Cannot find module '@google-cloud/functions-framework/testing' or its corresponding type declarations.ts(2307) in modern times, the fix is here as dimip1606 calls out. I note that "moduleResolution": "nodenext" needs to go under compilerOptions, not at the top level.

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.

None yet

3 participants