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

RFC - Testing API Side Functions and Webhooks with Scenarios #2765

Closed
5 of 8 tasks
dthyresson opened this issue Jun 8, 2021 · 13 comments · Fixed by #3207
Closed
5 of 8 tasks

RFC - Testing API Side Functions and Webhooks with Scenarios #2765

dthyresson opened this issue Jun 8, 2021 · 13 comments · Fixed by #3207

Comments

@dthyresson
Copy link
Contributor

dthyresson commented Jun 8, 2021

Missing from the RedwoodJS testing suite is a way to test api side functions and webhooks with the same scenario-based approach used with api side services. The goal is to follow the same patterns in api/web testing such that testing functions/webhooks is familiar and looks the same.

Some considerations and decisions need to be made to support function and webhook testing.

I have a proof of concept working in https://github.com/redwoodjs/redwood-challenges/tree/main/api/src/functions/__tests__

Tasks

  • Determine directory structure for tests and fixtures
  • Add test and scenarios to function generators
  • Add fixture helpers to CRWA or make available "in some way" to tests
  • Helpers to construct the function handler's event and context
  • Jest config to filter out scenarios
  • Ensure tests and scenarios are not included in api deploy distribution (Netlify, Vercel, Render, etc etc)
  • Convert to JS
  • Documentation

Proof of Concept Directory Structure

-- api
---- src
------ functions
-------- __tests__
---------- __fixtures__
------------h buildContext.ts
------------h buildEvent.ts
------------x dataForFunctionName.ts
----------s functionName.scenarios.ts
----------t functionName.test.ts
----------s webhookName.scenarios.ts
----------t webhookName.test.ts
--------f functionName.ts
--------f webhookName.ts

f - is a function/webhook
h - are helpers in fixtures 
x - fixture data needed for tests apart from scenarios (ie, response data from third-party apis)
s - scenarios (if need to standup model data in case using services)

Updated API Jest Config

Add filter to ignore scenarios in testPathIgnorePatterns.

const dotenv = require('dotenv-defaults')
const { getConfig } = require('@redwoodjs/core')

dotenv.config({ defaults: '../env.defaults' })
process.env.DATABASE_URL = process.env.TEST_DATABASE_URL

const config = getConfig({ type: 'jest', target: 'node' })
config.displayName.name = 'api'
config.testPathIgnorePatterns = [
  '/node_modules/',
  '__fixtures__',
  '.scenarios.',
]
module.exports = config

buildEvent

A function handler needs an event and context, typically lambda (APIGatewayProxyEvent) and some context.

buildEvent takes a payload, and constructs the necessary param to be an expected APIGatewayProxyEvent - include headers, queryParams, httpMethods, etc.

import type {
  APIGatewayProxyEvent,
  APIGatewayProxyEventHeaders,
} from 'aws-lambda'

interface BuildEventParams extends Partial<APIGatewayProxyEvent> {
  payload?: string
  signature?: string
  signatureHeader?: string
  headers?: APIGatewayProxyEventHeaders
}

// See: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/aws-lambda/trigger/api-gateway-proxy.d.ts
export const buildEvent = ({
  payload,
  signature,
  signatureHeader,
  queryStringParameters = null,
  httpMethod = 'GET',
  headers = {},
  path = '',
  isBase64Encoded = false,
  ...others
}: BuildEventParams): APIGatewayProxyEvent => {
  if (signature && signatureHeader) {
    headers[signatureHeader.toLocaleLowerCase()] = signature
  }

  const body = isBase64Encoded
    ? Buffer.from(payload || '').toString('base64')
    : payload

  return {
    body,
    headers,
    multiValueHeaders: {},
    isBase64Encoded,
    path,
    pathParameters: null,
    stageVariables: null,
    httpMethod,
    queryStringParameters,
    requestContext: null,
    resource: null,
    multiValueQueryStringParameters: null,
    ...others,
  }
}

buildContext

At moment, is a stub, but here can sent identity and clientContext info. See: https://docs.aws.amazon.com/lambda/latest/dg/nodejs-context.html

import type { Context } from 'aws-lambda'

export const buildContext = (): Context => {
  const context = {} as Context

  return context
}

Example Function Test

The entryHtml.test.ts uses scenario data entryHtml.scenarios.ts to create entries one of which is find-me-by-id-one.

Here buildEvent passes in a query string param corresponding to the scenario find-me-by-id-one.

The function fetches the entry from a service and returns a body -- which is checked from an expecrted result.

//entryHtml.test/ts
import { handler } from '../entryHtml'

import { buildEvent } from './__fixtures__/buildEvent'

describe('EntryHTML function', () => {
  scenario('Returns decoded content from an Entry', async () => {
    const entryHtmlEvent = buildEvent({
      queryStringParameters: {
        id: 'find-me-by-id-one',
      },
    })

    const result = await handler(entryHtmlEvent, null)
    expect(result.statusCode).toBe(200)
    expect(result.body).toEqual(expect.stringContaining('Welcome to RedwoodJS'))
  })

  scenario('Returns 404 for an entry that cannot be found', async () => {
    const entryHtmlEvent = buildEvent({
      queryStringParameters: {
        id: 'baddy-mcbadface',
      },
    })

    const result = await handler(entryHtmlEvent, null)
    expect(result.statusCode).toBe(404)
  })
})

Example Webhook Test

The ``pullRequestWebhook.test.tsshows howbuildEvent` can be used to mock the event with a signature.

This also uses nock to intercept a third-party api (a got request) that the webhook also calls.

The scenario sets up necessary database records for the function to persists the info received from the webhook.

The encodedGitHubFileContent is fixture data that is needed to verify the mock response from the nock intercept.

// pullRequestWebhook.test.ts
import type { APIGatewayEvent, APIGatewayProxyEventHeaders } from 'aws-lambda'

import nock from 'nock'

import { signPayload } from '@redwoodjs/api/webhooks'
import { handler } from '../pullRequestWebhook'
import { db } from 'src/lib/db'
import { logger } from 'src/lib/logger'

import { buildEvent } from './__fixtures__/buildEvent'

import { buildContext } from './__fixtures__/buildContext'

import { validMergedPullRequest } from './__fixtures__/validMergedPullRequest'

import { encodedGitHubFileContent } from './__fixtures__/encodedGitHubFileContent'

const mockPullRequestWebhook = ({ payload, invalidSecret = false }) => {
  const signature = signPayload('sha256Verifier', {
    payload: JSON.stringify(payload),
    secret: invalidSecret
      ? 'invalid'
      : process.env.GITHUB_PULL_REQUEST_WEBHOOK_SECRET,
  })

  const headers = {} as APIGatewayProxyEventHeaders
  headers['x-github-event'] = 'pull_request'

  const event = buildEvent({
    payload: JSON.stringify(payload),
    signature,
    signatureHeader: process.env.GITHUB_WEBHOOK_SIGNATURE_HEADER,
    headers,
  })

  const context = buildContext()

  return { event, context }
}

describe('valid scenario pullRequestWebhook', () => {
  beforeAll(() => {
    nock('https://github.com')
      .get('/dthyresson/redwood-webhook-test/pull/4/files')
      .reply(200, encodedGitHubFileContent)
  })

  scenario('verifies the event payload', async () => {
    logger.debug(
      { id: validMergedPullRequest.pull_request.id },
      'realWorldMergedPullRequest'
    )

    const { event, context } = mockPullRequestWebhook({
      payload: validMergedPullRequest,
    })

    const response = await handler(event as APIGatewayEvent, context)

    expect(response.statusCode).toEqual(201)
  })

  scenario('creates an entry', async () => {
    const { event, context } = mockPullRequestWebhook({
      payload: validMergedPullRequest,
    })

    const _response = await handler(event as APIGatewayEvent, context)

    const entryCount = await db.entry.count()

    expect(entryCount).toEqual(1)
    })

  scenario(
    'creates an entry associated to the challenge set in the label',
    async () => {
      const { event, context } = mockPullRequestWebhook({
        payload: validMergedPullRequest,
      })

      const _response = await handler(event as APIGatewayEvent, context)

      const challenge = await db.entry.findFirst({ take: 1 }).challenge()

      expect(challenge.slug).toEqual('splash-page')
    }
  )

  scenario('creates an entry with the payload content', async () => {
    const { event, context } = mockPullRequestWebhook({
      payload: validMergedPullRequest,
    })

    const _response = await handler(event as APIGatewayEvent, context)

    const entry = await db.entry.findFirst({ take: 1 })

    expect(entry.content).toEqual(encodedGitHubFileContent.content)
  })
})
...
@dthyresson
Copy link
Contributor Author

dthyresson commented Jun 9, 2021

What about a directory structure of

-- api
---- src
------ functions
---------__fixtures__
----------h buildContext.ts
----------h buildEvent.ts
----------x dataForFunctionName.ts
---------__tests__
----------s functionName.scenarios.ts
----------t functionName.test.ts
----------s webhookName.scenarios.ts
----------t webhookName.test.ts
--------f functionName.ts
--------f webhookName.ts

This way there is no nesting of fixtures in tests -- and fixturs comes pre-setup with the two helpers and ready for custom fixtures.

@thedavidprice
Copy link
Contributor

I don't believe the initial convention design of api/src/functions considered a future with test and scenario files. Maybe we should revisit that and consider each function being in a directory of the same name a la web components.

Also should consider implication per this Issue if we ever want to support nesting (either just as a convention or also "nested" end-point structure) #849

@dthyresson
Copy link
Contributor Author

dthyresson commented Jun 9, 2021

I don't believe the initial convention design of api/src/functions considered a future with test and scenario files. Maybe we should revisit that and consider each function being in a directory of the same name a la web components.

Also should consider implication per this Issue if we ever want to support nesting (either just as a convention or also "nested" end-point structure) #849

@dac09 and I had thought about nesting and that's our preferred approach -- to have it look much like services and web/components etc ... and we looked at that very same issue #849 as well.

But it wasn't as easy as we all thought.

Still exploring options.

@dac09
Copy link
Collaborator

dac09 commented Jun 15, 2021

Decision from core team meeting ✅

  • we will follow the same convention as services, and create a directory to house scenarios, test and your function file
  • at build time, we will rename e.g. functions/helloWorld/helloWorld.js to functions/helloWorld/index.js - to maintain convention for netlify & vercel [if necessary]
  • if you add a folder, just to group them, we will not flatten them
    e.g.
└── webhooks
    ├── paddle
    │   └── paddle.js
    └── stripe
        └── stripe.js

Your functions will be reachable on /api/functions/webhooks/paddle and api/functions/webhooks/stripe

Outstanding things we need to verify:

  • vercel also supports folderName/index.js
  • how do we need to change devserver/apiserver to support this behaviour

@Tobbe
Copy link
Member

Tobbe commented Jun 15, 2021

  • at build time, we will rename e.g. functions/helloWorld/helloWorld.js to functions/helloWorld/index.js - to maintain convention for netlify

Netlify already supports functions/helloWorld/helloWorld.js

This is from their docs

For example, with a configured functions directory of my_functions and a target function endpoint name of hello, you could save the function file in one of the following ways:

  • my_functions/hello.js
  • my_functions/hello/hello.js
  • my_functions/hello/index.js

See the second example in the list: my_functions/hello/hello.js

@dac09
Copy link
Collaborator

dac09 commented Jun 15, 2021

Thanks @Tobbe - need to improve my reading skillz. I think once we verify how vercel does it, we can decide what to do.

Our dev/api server needs to change, but thats not a big deal.

Index seems to me to be the most likely to be compatible, but I couldn't find the equivalent section of the docs for that on vercel.

@dthyresson
Copy link
Contributor Author

Something to consider is if the dev server can serve up the function in the same way as an endpoint.

@thedavidprice
Copy link
Contributor

Not to overdo the research ideas, but it possible we should look into Azure functions as well.

Also, I'm assuming Netlify === AWS Lambdas (direct deploy), but maybe not?

@dac09 dac09 self-assigned this Jun 22, 2021
@dac09
Copy link
Collaborator

dac09 commented Jun 22, 2021

Vercel does not support the nested functions behaviour. Functions in folders are simply ignored. In order to maintain compatibility (and to make it easy to reason about) - the best course of action after discussion with @peterp is to do something like this:

Input file tree

api/src/
├── functions
│   ├── graphql.js
│   └── helloWorld
│       └── helloWorld.js

Gets transformed to

Dist output tree

api/dist/
└── functions
    └── helloWorld.js # this file basically rexports handler in _build/helloWorld/helloWorld.js
└── _build
    ├── helloWorld
    │   ├── helloWorld.js

@dthyresson
Copy link
Contributor Author

@dac09 Another scenario to add to your test cases for Netlify are the On-Demand Builders:

https://docs.netlify.com/configure-builds/on-demand-builders/

const { builder } = require("@netlify/functions")

async function myfunction(event, context) {
    // logic to generate the required content
}

exports.handler = builder(myfunction);

Since it wraps the handler.

@dac09
Copy link
Collaborator

dac09 commented Jun 22, 2021

Some more important info from investigation, although Netlify supports the folders as per their docs, what they do not let you do is group functions into a folder.

So in this example:

api/dist/
├── functions
│   ├── checking
│   │   ├── checking.js
│   │   ├── other.js
│   ├── group
│   │   ├── hey.js

☑️ myapp.com/api/checking runs checking.js
⚠️ myapp.com/api/checking/other just runs checking.js
🛑 myapp.com/api/group/hey 404s

@Tobbe
Copy link
Member

Tobbe commented Jun 22, 2021

Vercel does not support the nested functions behaviour.

I actually think they do. I wrote this in a previous issue (#849)

To get that to work on Vercel I had to do some tricks with the vercel.json config files and the routing in there.

File structure
image

vercel.json
image

With that I can access:
hello.js at url.tld/api/hello
nested_function.js at url.tld/api/hello/nested_function

So you do have to map it all out in vercel.json

So, yeah, it is possible. Just putting it out there. Do what you want with the info :)

@dac09
Copy link
Collaborator

dac09 commented Jun 22, 2021

Yeah ofcourse, with redirects everything is possible ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants