[v0.36] Adds ability to test api functions

[dac09]

Closes #2765
Closes #849

What does this do?

This PR adds built-in capability to RW, to test your api functions (i.e. functions generated with yarn rw g function).

This is a non-breaking change 🍾 - all existing functions will still work as is.

Changes include:

• Various build process changes to support nesting functions in a folder, along with their tests and scenarios. This follows the same convention as most other redwood generators
• generate function now generates test and scenario files
• Adds helper functions to @redwoodjs/testing - mockHttpEvent, mockContext and mockSignedWebhook to use in your unit tests, to make it a lot cleaner
• Changes @redwoodjs/testing package to expose testing utilities from @redwoodjs/testing/web and @redwoodjs/testing/api so it doens't interfere with each other
• Uses module mapper in jest configs to make the above change non-breaking for existing projects

A note on nesting functions:

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

api/src/functions/hello.js
api/src/functions/hello/hello.js
api/src/functions/hello/index.js

---

Outstanding

• Fix on Windows
• Figure out what we're doing about importing from @redwoodjs/testing because this is currently breaking storybook

[dthyresson]

Looks great.

So, I think an example test might be something close to:

import { handler } from '../entryHtml'
import { mockHttpEvent } from '....

describe('EntryHTML function', () => {
  scenario('Returns decoded content from an Entry', async () => {
    const entryHtmlEvent = mockHttpEvent({
      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 = mockHttpEvent({
      queryStringParameters: {
        id: 'entry-does-not-exist',
      },
    })

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

[peterp]

This is looking super promising @dac09. I would love to see some of it re-architectured (I've left some comments)

I get the feeling that splitting this PR into multiple PR's would make is easier to have conversations about it because there's a lot going on here! My preference would be to focus on the proxy api server submodule functions first.

Let me know what you think, and please let me know if there's anything that's not clear.

[peterp]

I have some problems with:

a) the name of this function because it doesn't tell you what it's actually doing: it looks like it's generating files to supported "api server functions" that are nested in subdirectories.

So let's give this concept a name that describes what it's doing? What are we trying to solve? We want people to be able place "api server functions" into "subfolders"

generateProxyModulesForApiServerSubfolderFunctions

I told you I was an objective-c developer in my earlier days 🤪

b) the place where this function is invoked. It's not actually a transpilation-step, but rather a post-build+generation step that is focused on a narrow set of files, the ones return by findApiServerFunctions.

You should:

1. Remove this code from "prebuild", and rather invoke it afterwards in the "buildApi" function.
2. Filter out the functions that are not important; you're only concerned with subfolder function files, and a nice way of doing this that is nicely testable is adding a isApiSubfolderFunction tester. (see files.ts::isApiFunction)
3. Make it do all the work in this single function: iterate over the files, figure out the paths, and write the content.

[dac09]

These are good points, and I totally don't like how this code reads either. I'm not sure I fully understand this yet, but once I start playing around with it maybe it will!

I'll see if I can implement your suggestions - however, this did build on the existing convention in the file before I think i.e. we were getting the path and content inline within the transpliation-step.

[dac09]

Ok, so I think I understand the suggestion here now. Thanks for the detailed review Peter! Some stuff to discuss:

1. Remove this code from "prebuild", and rather invoke it afterwards in the "buildApi" function.

I can see your logic here - but let try and convince you this is the better place for it. If the purpose of prebuild is to "remove all redwood magic", i.e. leave es6 js files that will just run, I think it makes perfect sense to leave it in prebuild. The way I see it, this step allows functions to run on all our supported providers (netlify, vercel, render, self-hosted) - and they all behave consistently.

Without generating these proxy functions, what we get is code that won't quite work on all providers, because they do not support nesting consistently. I think it also is neater that the esbuild step does only one thing - transpiles from es6 to the target file type (the function name even says transpileApi).

2. Filter out the functions that are not important; you're only concerned with subfolder function files, and a nice way of doing this that is nicely testable is adding a isApiSubfolderFunction tester. (see files.ts::isApiFunction)

Just confirming the suggestion here, in pseudo-code, should the suggested implementation look like this?

buildApi() {
	const nonNestedApiFiles = findApiFilesWithoutNestedFunctions()
	const nestedFunctions = findNestedFiles()

	const allPrebuiltFiles = [...preBuildApiFiles(nonNestedApiFiles), ...preBuildNestedFunction(nestedFunctions)]

	return transpileApi(allPrebuiltFiles)
}

3. Make it do all the work in this single function: iterate over the files, figure out the paths, and write the content.

If we must split the lookup of api files, question here would be there's 2 ways we could group the api files:

• Set 1:
  a) All functions
  b) All other files

or

• Set 2:
  a) Non-nested functions + all other files
  b) Nested functions only

Personally I prefer Set 1 - but totally open to other ideas.

[peterp]

I hear you, but I disagree.

I think there are two concepts:

1. Transpile the user's RedwoodJS code in standard JavaScript.
2. Supported serverless functions in subdirectories by moving the subdirectories and generating a proxy module.

In a broad sense I agree that they're "enabling redwood magic," but I still think they're doing two separate things to achieve that goal.

Here's a thought experiment:

A year from now we decided that "no on uses serverless functions in subdirectories anymore!" And want to delete it... we would have to scratch around in the transpilation code (that we don't want to remove), and worse yet, the return type is modified, so we would have to check all the places that the transpilation code is invoked to make sure we're doing a good job.

When introducing new functionality I try to determine what it would take to delete that code if the functionality was no longer required. The idea is that deleting code should be much easier than fixing bugs and adding new functionality. If deleting it is difficult, then fixing bugs and adding functionality is going to be really difficult because it's probably too tightly coupled, and requires you to understand more than what the functionality introduces.

Alternatively if you added it in the way I suggested above then deleting it would be removing the invocation of a single function and removing that function. (Also super easy to test.)

[peterp]

If you want we can pair and refactor this implementation.

[dac09]

Refactored :) - theres is a little down side, which is that I'm having to move files. But if we wanted to undo this, you literally have to remove one line.

[peterp]

I'm not exactly following why you're placing it in an _build directory?

[peterp]

E.g.: This comment tells me what you're doing, but I don't understand the motivation.

[dac09]

Same as before:

Here's the concept: #2765 (comment)

Idea actually came from our discussion on this few weeks ago ;)

[peterp]

pathForProxyApiSubfolderFunction

[peterp]

I actually think we should split the these into @redwoodjs/testing/api and @redwoodjs/testing/web.

I've had a helleva time trying to figure out why storybook wasn't working because I accidentally imported a NodeJS library.

[dac09]

Yeah this code is organised in a difficult way.

Personally, I'd prefer not to make this change right now because then it becomes a breaking change

[dac09]

Done, but using module mapper so its not a breaking change.

[dthyresson]

Now I learned about Exclude.

[dac09]

Converting to draft - till we decide on where to place the api testing helpers. Currently this is causing storybook to fail.

Also, windows tests...

Update: Resolved both.

[peterp]

Some small suggestions!

[thedavidprice]

Would be RadSauce to get this into v0.36 — seems possible judging from the current review status. Just keep me posted either way.

[dthyresson]

Would be RadSauce to get this into v0.36 — seems possible judging from the current review status. Just keep me posted either way.

I just finished up documenting actual code examples for functions and webhook testing using this PR -- and it worked great.

Draft at moment with just the code examples here: https://github.com/redwoodjs/redwoodjs.com/pull/774

Copy content around the examples to come soon.

I wanted to get the examples working first before describing.

[thedavidprice]

This is looking amazing.

• Destroy function is working great!
• can't get the generated Function test to pass

Reproduce:

• yarn rw g function foo
• yarn rw test

Here's the error for the specific test:

 FAIL   api  api/src/functions/foo/foo.test.ts
  ● Test suite failed to run

    Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' from '../node_modules/@redwoodjs/testing/dist/MockProviders.js'

    Require stack:
      /Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/testing/dist/MockProviders.js
      /Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/testing/dist/customRender.js
      /Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/testing/dist/index.js
      src/functions/foo/foo.test.ts

      at Resolver.resolveModule (../node_modules/jest-resolve/build/resolver.js:311:11)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/MockProviders.js:32:5)

NOTE: I was using the yarn dev --cwd [path to test project] test for testing. It's possible it didn't set the correct paths for dist/

Do I need to rwfw project:sync instead?

[thedavidprice]

@dac09 and @dthyresson As a next step once this is merged, let's add this to the existing E2E. Would be great to add along with this PR, but I don't want to hold it up.

[dac09]

NOTE: I was using the yarn dev --cwd [path to test project] test for testing. It's possible it didn't set the correct paths for dist/

Do I need to rwfw project:sync instead?

@thedavidprice yeah I don't know how that command works really :). Here's an example with Rufus https://s.tape.sh/m3szgH9b

[dac09]

All comments addressed!

[peterp]

🦦

[thedavidprice]

Ship it!!