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

Test Scenarios #1465

Merged
merged 47 commits into from
Jan 6, 2021
Merged

Test Scenarios #1465

merged 47 commits into from
Jan 6, 2021

Conversation

cannikin
Copy link
Member

@cannikin cannikin commented Nov 10, 2020

After a good couple of hours working with @peterp and @mojombo we think we came up with a pretty clean way to integrate scenarios (formerly "fixtures") into the test workflow! This PR has a bunch of backstory and reads like a thriller so grab a warm drink and snuggle up with a blanket!

Definitions

  • scenario — data that is pre-set into the database at the start of your test, and removed afterwords

The Problem

Services are generally going to be accessing your database to retrieve/create/update/delete data. In most cases you'll want some data in the database at the start of the test. You could insert this data yourself:

it('returns a list of posts', async () => {
  const posts = [
    { title: 'First post',  body: 'Foobar' },
    { title: 'Second post',  body: 'Bazqux' }
  ]
  for (const post in posts) {
    await db.post.create({ data: post })
  }

  const dbPosts = db.post.findMany()
  expect(dbPosts.length).toEqual(posts.length)

  await db.post.deleteMany()
})

8 of the 10 lines in this test (80%) is just putting data into the database and cleaning up afterwords! And if you have another test that wants to use this data you have to write all the setup again. Also, the data in the posts variable is just the plain object that I created, not the actual data in the database which will include additional fields like id or createdAt which are set to default values in the database itself.

So the next logical step is to just put the data insertion into a beforeEach() and the cleanup into an afterEach() and actually return the data as it appears in the database:

const POSTS = [
  { title: 'First post',  body: 'Foobar' },
  { title: 'Second post',  body: 'Bazqux' }
]

const posts = []

beforeEach(() => {
  for (const post in POSTS) {
    posts.push(await db.post.create({ data: post }))
  }
})

afterEach(() => {
  await db.post.deleteMany()
})

it('returns a list of posts', async () => {
  const dbPosts = db.post.findMany()
  expect(dbPosts.length).toEqual(posts.length)
})

At least the test (in the it() function) is now dealing with just the test data, which is an improvement. But I'd still have to write these beforeEach and afterEach functions for each services test and their content is essentially the same:

  1. Insert some data
  2. Delete some data

The next logical step is to have Redwood do the insertion and cleanup for me. That's where this PR comes in.

The Solution

Here's what we came up with. What if the test suite, the entire thing, could look like this:

import { comments } from './comments'

describe('comments', () => {
  it('returns a list of comments', async (scenario) => {
    const list = await comments()
    expect(list.length).toEqual(scenario.comment.length)
  })
})

Where the scenario argument is the sample data you inserted into the database. Let Redwood worry about seeding it and removing it afterwords. Where does the actual scenario data live? Just like the mocks solution we came up with for stories, it lives in its own file, in this case comments.scenarios.ts:

export const standard = {
  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',
        },
      },
    },
  },
}

The nested structure contains:

  • comment - model name
    • rob - name of the fixture
      • name, body, etc. - the actual data that will be inserted into the comment model

You can insert data into nested models by using either the prisma create syntax (as in the example above) or declaring another model at the first level of the data structure:

export const standard = {
  comment: {
    rob: {
      // ...
    },
  },
  post: {
    first: {
      title: 'First Post',
      body: 'Lorem ipsum...'
    },
  },
}

When you get your scenario object in your test, it's an object with the same structure as what you created above, but after it's been inserted in the database, so it includes data in the fields that the database itself sets, like id and createdAt:

scenario.comment.rob.name
scenario.comment.rob.id
scenario.comment.rob.createdAt

To get this working, we create our own test function that:

  1. Adds the scenario data to the database
  2. Runs the test
  3. Removes the records that were created

We do this in our custom function named scenario():

scenario('returns a list of comments', async (scenario) => {
  const list = await comments()
  expect(list.length).toEqual(scenario.comment.length)
})

Just like with the component mocks, we default to a scenario named standard. As long as you export that in your scenarios file, we'll use it by default. If you want to use another named scenario, export it from your *.scenarios.js file and pass that name, as a string, as the first argument to scenario():

scenario('withoutPosts', 'returns a list of comments', async (scenario) => {
  const list = await comments()
  expect(list[0].post).toEqual(null)
})

This lets us preserve the existing it() and test() functions and make it clear to users that if you want to use fixtures, use our new function instead.

Little history: the naming comes from a Ruby gem that @mojombo wrote years ago for Rails called fixture-scenarios.

Todo

  • Update the generators to include scneario files and tests that use it
  • VS Code hints aren't quite working with scenario...there were a couple changes made to some index.d.ts, global.d.ts and eslint-config files with Peter's help but it didn't fix the issue. He was going to look into that some more.

@cannikin
Copy link
Member Author

cannikin commented Nov 14, 2020

@peterp After a couple of extensive coding sessions with @mojombo we made some updates! Trying to override it and test was causing all kinds of problems with async tests so we said screw it and just went with our own. We just named it scenario() and it accepts an optional first parameter, which is the name of the scenario to use, otherwise it defaults to one called "standard". I updated the code snippets above to use the function name.

This leaves out the typescript stuff though. The idea was that when declaring your scenario objects in *.fixtures.js you'd call a function instead of just create an object, and that function would worry about the types. Unless we were mistaken, Tom and I both thought we'd need to somehow re-generate the types on every prisma generate in case the user changed something about the schema somewhere. We sidestepped that problem and hoped you would have some idea of that would work. 😬

import { scenario } from '@redwoodjs/testing'

export const standard = scenario({
  comment: {
    rob: {
      name: 'Rob'
      // ...
    }
  }
})

@peterp
Copy link
Contributor

peterp commented Nov 16, 2020

@cannikin This is look great, I think I can dynamically create those prisma model types at run-time. So we would default to the standard scenario, but a user could export multiple scenarios and pick the scenario to run?

Could you include a comment of how to use the new function?

@cannikin
Copy link
Member Author

So we would default to the standard scenario, but a user could export multiple scenarios and pick the scenario to run?

Yup! The scenario takes an optional first argument. Which seems weird, I know, but after @mojombo and I tried a bunch of different combinations, the first position had the most natural feel to it: the name was right next to the word "scenario" and conveys understanding better.

The two ways to call the method:

  1. Without a scenario name, which makes it a drop-in replacement for it or test that will use the standard scenario
  2. With a scenario name as the first argument, which is the same name as an export in the *.fixtures.js file
// users.fixtures.js

export const standard = {
  users: {
    rob: {
      name: 'Rob Cameron'
    }
  }
}

export const withProfiles = {
  users: {
    rob: {
      name: 'Rob Cameron',
      profile: {
        create: {
          username: 'cannikin'
        }
      }
    }
  }
}

// users.test.js

import { users } from './users'

// standard scenario
scenario('returns a list of users', async (fixtures) => {
  const usersList = await users()
  expect(usersList[0].name).toEqual(fixtures.users.rob.name)
})

// named scenario
scenario('withProfiles', 'returns a list of users and their profiles', (fixtures) => {
  const usersList = await users({ includeProfiles => true })
  expect(usersList[0].profile.username).toEqual(fixtures.users.rob.profile.username)
})

@cannikin
Copy link
Member Author

@peterp I wasn't sure how to construct the type in https://github.com/redwoodjs/redwood/pull/1465/files#diff-dc00f5226046500fcbb89faee6cb2093feffa41a14bd9e644064267b4dd1818d since the second parameter could be a function OR a string, and if it's a string then the third parameter must be present, and be a function. TYPESCRIPT

@cannikin cannikin marked this pull request as ready for review November 16, 2020 21:53
@cannikin cannikin requested a review from peterp November 16, 2020 21:54
@cannikin
Copy link
Member Author

Maybe this code should go into a separate JS file somewhere, instead of everything being crammed into jest.setup.js?

@github-actions
Copy link

github-actions bot commented Nov 16, 2020

@Tobbe
Copy link
Member

Tobbe commented Nov 16, 2020

@cannikin Don't know if it applies here, but generally, what you're asking would be solved like this

function foo(first: string, second: () => void): void;
function foo(first: string, second: string, third: () => void): void {
  // implementation
}

@cannikin
Copy link
Member Author

@peterp I updated the eslint-config stuff to add the scenario function but I get that same any popup when VS Code tries to give a hint:

image

@cannikin cannikin marked this pull request as draft November 17, 2020 21:50
@cannikin
Copy link
Member Author

Converted back to draft! Talked with @mojombo and we're going to drop the "fixtures" nomenclature and just stick with "scenarios." Jumping back and forth was getting confusing and I found myself making an aside block in the Tutorial II to try and explain the difference myself and having trouble.

So the files will become *.scenarios.js and the argument you pass to the scenario() function will also be called scenario so you can reference the fields inside as scenario.comment.rob.name. One word to rule them all!

@cannikin cannikin changed the title Test Fixtures Test Scenarios Nov 17, 2020
@peterp
Copy link
Contributor

peterp commented Nov 18, 2020

@cannikin Let me know when you want me to try add some types to this!

@cannikin
Copy link
Member Author

@peterp You should be able to go ahead? I'm working on the generator right now so that it creates the *.scenarios.js file along with the service. But the object I'm creating will match the types that Prisma creates, so you should be able to create that function that'll wrap the scenario JSON (also called scenario() I think?). In theory, the *.scenario.js files would end up looking something like:

import { scenario } from '@redwoodjs/scenario'

export const standard = scenario({
  post: {
    first: {
      title: 'First Post',
      body: 'Lorem ipsum dolar sit amet',
    },
  },
})

I think?

@cannikin
Copy link
Member Author

Holy cow @peterp I got Storybook to not blow up with hasRole()! See 8460e77

I don't understand why only the parts exported from AuthContext exist within Storybook, while everything in AuthContextInterface is what's actually available from useAuth() in the context of a real app?

Here's a console log of useAuth() within a story:

image

Here's the same output from the dev server:

image

@cannikin
Copy link
Member Author

Oh boy @peterp you're either going to love or hate this one. ;)

After several hours of working with @mojombo, @Tobbe, @dthyresson and @clairefro we were able to mock the currentUser in a story!

mockCurrentUser({
  email: 'test@example.com',
  roles: ['moderator'],
})

Just insert that at the top of a story and you're good to go.

How we got that working...it's either genius or insanity. We wrapped the existing mock providers in an <AuthProvider> along with a new mock auth client because the fakeUseAuth didn't seem to do anything: https://github.com/redwoodjs/redwood/pull/1465/files#diff-a2640d09eed5d305774c16eadddcab67e7e61d3ed209a2c879f9b1359c777ce5

We mock the currentUser to be null in StorybookProvider and then if you call mockCurrentUser yourself it'll get overridden with whatever you want.

There was some weirdness with declaring a global that wasn't a const... typescript made us use var, but eslint doesn't like that, so we added an ignore: https://github.com/redwoodjs/redwood/pull/1465/files#diff-dc00f5226046500fcbb89faee6cb2093feffa41a14bd9e644064267b4dd1818d

I can walk you through the other stuff we did, let me know! I know you said you were looking into mocking currentUser and it looked pretty easy, so maybe your way was way more elegant than this, but it was a fun exercise. :)

@@ -35,5 +35,8 @@ export const StorybookProvider: React.FunctionComponent<{
return null
}

// default to a non-existent user at the beginning of each story
mockCurrentUser(null)
Copy link
Contributor

Choose a reason for hiding this comment

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

I usually like to be explicit for things like this.

Suggested change
mockCurrentUser(null)
window.mockCurrentUser(null)

Copy link
Contributor

Choose a reason for hiding this comment

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

Just so that people reading the code know that this is a global.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, problem. Putting window. or global. in front of that function throws an error at build time:

@redwoodjs/core: src/storybook/StorybookProvider.tsx(39,10): error TS2339: Property 'mockCurrentUser' does not exist on type 'Window & typeof globalThis'.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm assuming you're offline for the day and we wanna get 0.22 out, so I'll keep this as just mockCurrentUser(null) for now and we can revisit!

Copy link
Contributor

@peterp peterp left a comment

Choose a reason for hiding this comment

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

I left a few comments about making the code easier to debug for the user, but it looks good to me!

@cannikin cannikin merged commit 6823af3 into main Jan 6, 2021
@cannikin cannikin deleted the rc-text-fixtures branch January 6, 2021 20:59
@thedavidprice thedavidprice added this to the Next Release milestone Jan 6, 2021
dac09 added a commit to dac09/redwood that referenced this pull request Jan 12, 2021
…h-tokens

* 'main' of github.com:redwoodjs/redwood:
  Move whatwg-fetch from devDep to dep
  Adds mockCurrentUser() to api-side jest
  v0.22.1
  v0.22.0
  Revert "Configure ApolloProvider to always use fresh tokens  (redwoodjs#1609)" (redwoodjs#1611)
  Configure ApolloProvider to always use fresh tokens  (redwoodjs#1609)
  Ignore *.scenarios.* files. (redwoodjs#1607)
  upgrade prisma v2.12.1 (redwoodjs#1604)
  Test Scenarios (redwoodjs#1465)
  Use relative path to config stories location (redwoodjs#1509)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants