improve generated Service test template

[thedavidprice]

@RobertBroersma @peterp

The current generated Service test is not actually testing the generated Service code (i.e. not even if it exists):

// api/src/services/posts/posts.test.js

/*
import { posts } from './posts'
*/

describe('posts', () => {
  it('returns true', () => {
    expect(true).toBe(true)
  })
})

Given the recent MSW integration with test DB, could we update this to at least be meaningful?

And we do have two cases to consider if we'd like to handle both distinctly:

1. yarn rw g service post (no CRUD)

   
   // api/src/services/posts/posts.js

import { db } from 'src/lib/db'

export const posts = () => { return db.post.findMany() }

2. `yarn rw g service post --crud`
```js
// api/src/services/posts/posts.js

import { db } from 'src/lib/db'

export const posts = () => {
  return db.post.findMany()
}

export const post = ({ id }) => {
  return db.post.findOne({
    where: { id },
  })
}

export const createPost = ({ input }) => {
  return db.post.create({
    data: input,
  })
}

export const updatePost = ({ id, input }) => {
  return db.post.update({
    data: input,
    where: { id },
  })
}

export const deletePost = ({ id }) => {
  return db.post.delete({
    where: { id },
  })
}

[RobertBroersma]

I wrote this test a while ago in my playground repo to see if all our testing capabilities work well. I think it would cover everything for the CRUD case:

import { db } from 'lib/db'

import { posts, post, createPost, updatePost, deletePost } from './posts'

describe('Post Service', () => {
  it('posts returns all posts', async () => {
    const newPost1 = await db.post.create({
      data: {
        title: 'Post 1',
        body: 'Test',
      },
    })

    const newPost2 = await db.post.create({
      data: {
        title: 'Post 2',
        body: 'Test',
      },
    })

    expect(await posts()).toEqual([newPost1, newPost2])
  })

  it('post returns the post by id', async () => {
    const newPost = await db.post.create({
      data: {
        title: 'Test',
        body: 'Test',
      },
    })

    expect(await post({ id: newPost.id })).toEqual(newPost)
  })

  it('createPost returns newly created post', async () => {
    const input = {
      title: 'Post 1',
      body: 'Test',
    }
    const newPost = await createPost({ input })

    expect(newPost).toEqual(expect.objectContaining(input))
    expect(await db.post.findMany()).toHaveLength(1)
    expect(await db.post.findOne({ where: { id: newPost.id } })).toEqual(
      expect.objectContaining(newPost)
    )
  })

  it('updatePost returns updated post', async () => {
    const newPost = await db.post.create({
      data: {
        title: 'Post 1',
        body: 'Test',
      },
    })

    const input = {
      title: 'Updated Post 1',
      body: 'Updated Test',
    }

    const updatedPost = await updatePost({
      id: newPost.id,
      input,
    })

    expect(updatedPost).toEqual(expect.objectContaining(input))
    expect(await db.post.findMany()).toHaveLength(1)
    expect(await db.post.findOne({ where: { id: newPost.id } })).toEqual(
      expect.objectContaining(updatedPost)
    )
  })

  it('deletePost returns deleted post', async () => {
    const newPost = await db.post.create({
      data: {
        title: 'Post 1',
        body: 'Test',
      },
    })

    const deletedPost = await deletePost({ id: newPost.id })

    expect(deletedPost).toEqual(newPost)
    expect(await db.post.findMany()).toHaveLength(0)
    expect(await db.post.findOne({ where: { id: newPost.id } })).toEqual(null)
  })
})

For the non-crud I guess we could just take the first test?

[cannikin]

After playing with this I have a couple of ideas...

What about using the nomenclature of "fixtures" for sample data that's inserted in your database at test time. At the top of a service could be a constant containing fixture data related to the service that's being tested:

const FIXTURES = [
  {
    name: 'Test',
    email: 'test@sample.com',
    message: 'Test',
  },
]

Kind of feels similar to Cells where we have a constant at the top of the page that deals with "data".

And then we make available a createFixtures function that will do the dirty work of inserting those records into the database for the given model:

await createFixtures('contact', FIXTURES)

Which makes a single test pretty simple, but still clear what's happening:

it('returns a list of contacts', async () => {
  await createFixtures('contact', FIXTURES)
  const list = await contacts()

  expect(list.length).toEqual(FIXTURES.length)
  expect(list[0]).toEqual(expect.objectContaining(FIXTURES[0]))
})

This helps to extract away the labor of directly adding records to the database and you can focus on just the inputs/outputs of the service itself, which is all you should be testing anyway.

If you don't want that fixture data in there for a particular test (creating a new record, for example) you just don't call createFixture in that test. Conversely, if you want the same fixture data in all tests, then include the createFixture call in a beforeEach.

As part of the Tutorial II I'm going through this code now and realizing we need to update the generators to have all of these tests ready to go so I'd like to decide on a pattern soon so I can start building.

[cannikin]

Hmmmm I'm getting this error when trying to run a test that contains a one-to-many relationship:

  ● comments › returns a list of comments

    Invalid `prisma.post.deleteMany()` invocation:

      The change you are trying to make would violate the required relation 'CommentToPost' between the `Comment` and `Post` models.

      at $w.request (../node_modules/@prisma/client/runtime/src/runtime/getPrismaClient.ts:1258:15)
      at Object.<anonymous> (../node_modules/@redwoodjs/core/dist/configs/node/jest.setup.js:24:5)

I'm not deleting anything in my test though...is this some of our own code trying to reset the database at the end of a run? It doesn't seem to like doing that when tables are related. Maybe we need some way to define what order records should be deleted in?

Or do we skip all of that and just drop and recreate the database after each and every test? Because this seems like it could be a huge rabbit hole. :(

My test:

import { FIXTURES as POST_FIXTURES } from '../posts/posts.test'

export const FIXTURES = [
  {
    name: 'Test',
    body: 'Test',
    post: {
      create: POST_FIXTURES[0],
    },
  },
]

describe('comments', () => {
  it('returns a list of comments', async () => {
    createFixtures('comment', FIXTURES)
    const list = await comments()

    expect(list.length).toEqual(FIXTURES.length)
    expect(list[0]).toEqual(expect.objectContaining(fixturesWithPosts[0]))
  })
})

[RobertBroersma]

@cannikin We try to delete everything in the DB here: https://github.com/redwoodjs/redwood/blob/main/packages/core/src/configs/node/jest.setup.js

Rationale is that deleting everything in the DB takes way less time than dropping, recreating and migrating a new DB for every test.

That snippet has caused issues before already because it's quite hacky...

It would be great if Prisma included a command for this, but I don't think it's high priority: https://github.com/prisma/prisma/issues/742

I'm not sure if I'd use a magical createFixtures for something that's pretty straightforward to do with db.whatever.create, but I haven't done too many API tests tbh, and I know @peterp has been playing with some fixture stuff as well, so I'm gonna pass the baton over to him :D

[cannikin]

I was thinking about this more this morning while waiting to pre-order my iPhone!

Taking a page from Rails, what we if automatically wrote all fixtures to the database for you? For example, if you export a FIXTURES object we'd stick those in the database for you at the beginning of the test, using the name of the test file as the name of the model. This feels conceptually similar to the mocks in our stories: you define them once and they're available in all your stories for that component, you don't need to re-create them in each and every story.

This behavior is one of my favorite things about testing in Rails—I don't need to specify the data in each and every test from scratch, I just declare them once and they're always there ready to go. The vast majority of the time that's exactly what I want, and in the few cases where I want/need a different starting state in the database I'll make those changes in the setup of that specific test.

Rolling back the database at the end of a test is easy for a single table with Prisma's deleteMany() but unfortunately that breaks down if you have one-to-many or many-to-many relationships in your test data—you need to delete the "many" records first before you delete the "one." I wish Prisma had some way to define cascading deletes so they could do that for us (you can specify it manually in the database itself but it seems like a ridiculous amount of work). Maybe we could introduce a some sort of an export or helper method where you can give hints as to what order things need to be removed:

export const FIXTURE_ORDER = ['comments', 'posts']
// or
teardown(['comments', 'posts'])

...and then we call deleteMany() on those models in that order. Otherwise we just deleteMany() the model that has the same name as the test file.

Opting out of all of this behavior would be easy enough: don't export a FIXTURES object (or whatever we wanted to call it).

[cannikin]

Yeah that code that tries to clean up the database isn't going to work: we need to delete records in a certain order (the many of the one-to-many side, for example). Prisma enforces those relationships with foreign keys constraints that won't let the records be deleted in the wrong order.

[cannikin]

Do tests inside a single test file run simultaneously as well? I'm seeing all kinds of weird behavior that I can't explain. We need to discuss on Tuesdays...testing services (more than one service) is BRUTAL right now.

Even the output is confusing...it shows FAIL next to the comments test, but the two failures are listed as posts tests, then at the end it has a PASS next to posts.test.js WHAT IS HAPPENING

[RobertBroersma]

Do tests inside a single test file run simultaneously as well? I believe so

I have seen these kinds of strange things before where passing tests result in errors and whatnot. It's one of the reasons I'm balding I think.

[cannikin]

Worked on some stuff with @mojombo this afternoon and I'm recording this here for posterity... another possible way to define fixtures. Uses the target model as the root key in the object (comment) and then gives each fixture a name with the second level key:

export const FIXTURES = {
  comment: {
    rob: {
      name: 'Rob',
      body: 'This is my comment',
      post: {
        create: {
          title: 'First Post',
          body: 'Lorem ipsum',
        },
      },
    },
    tom: {
      name: 'Tom',
      body: 'Also a great comment',
      post: {
        create: {
          title: 'Second Post',
          body: 'Dolar sit amet',
        },
      },
    },
  },
}

This would allow us to:

1. automate creating the fixtures because we can derive the model name from the object
2. reference the fixtures by name in the tests

it('retrieves comments', async () => {
  const data = await comments()
  expect(data[0].name).toEqual(FIXTURES.comment.rob.name)
})

Maybe rather than referencing the FIXTURES directly we can create a fixtures function which will reference the fixture data after being added to the database so that we can access any fields that were created by defaults in the database:

it('retrieve a single comment`, async () => {
  const data = await comment({ id: fixtures().comment.rob.id })
  // id is not present the fixture but `fixtures()` returns the result of the record being added to the database
  expect(data.id).toEqual(fixtures().comment.rob.id)
  // same with createdAt, it's defaulted to "now" by prisma in the DB
  expect(data.createdAt).toEqual(fixtures().comment.rob.createdAt)
})

Oh snap, now we're on to something...

[cannikin]

IT'S HAPPENING https://github.com/redwoodjs/redwood/pull/1465