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

Run Jest tests in sequence #1447

Merged
merged 3 commits into from
Nov 10, 2020
Merged

Run Jest tests in sequence #1447

merged 3 commits into from
Nov 10, 2020

Conversation

cannikin
Copy link
Member

@cannikin cannikin commented Nov 6, 2020

Jest will run all test files simultaneously by default. This means it's impossible to test services—any database records created in them can potentially be overwritten or wiped out by another file running at the same time.

This PR adds the --runInBand flag that makes Jest run each file one after the other. It's too bad we can't do this just for the api side but I don't see any way to specify that only certain files should run in band and not others.

Strangely enough this actually makes the services tests feel faster since you see them running and completing in sequence, instead of nothing happening for a couple of seconds and then them all completing at the same time.

Jest will run all test files simultaneously by default. This means it's impossible to test services—any database records created in them can potentially be overwritten or wiped out by another file running at the same time.

This PR adds the `--runInBand` flag that makes Jest run each file one after the other. It's too bad we can't do this just for the api side but I don't see any way to specify that only certain files should run in band and not others.
@Tobbe
Copy link
Member

Tobbe commented Nov 6, 2020

Reading this PR I had two thoughts

  1. Can't we have one db per test? So no matter what other tests might do to their db it won't affect any other test. I was thinking an in-memory db would be nice for this because it's so fast. Obviously I'm not the first one to think about this. @RobertBroersma mentioned it here [Testing] Get Jest up and running #502
  2. Could we run separate jest commands? I don't know the syntax, but having something like
    yarn rw test: jest --runInBand /api/* && jest /web/*
    

@RobertBroersma
Copy link
Contributor

Ah, good catch...

@Tobbe I would love to have in-memory database, but for now there are a few difficulties with it:

  1. Support in-memory SQLite connector prisma/prisma#732 Prisma doesn't support it (yet)
  2. If you're using SQLite-incompatible features like enum in your schema it won't work.
  3. Using a different kind of DB for testing vs production makes your tests less reliable (a trade-off I'd be happy to make, but some have expressed more concerns about this in [Testing] Get Jest up and running #502

I think your suggestion for running the commands separately makes sense in CI to save some time, but I'm not sure if that would work in watch mode.

Right now we make use of separate Jest configs for web and api, which are then run at the same time when going yarn rw test. I'm wondering if adding this option to the api config would run the API tests in band, and the web tests concurrently.

My hunch says no, but even if it does, my next hunch says that the web-side tests will also pick up the beforeEach from the api config, potentially destroying the database in the middle of any api test.

PS. I checked, and runInBand is only configurable in CLI, but according to jestjs/jest#3215 (comment) it can be set in the config, but it's undocumented.

@RobertBroersma
Copy link
Contributor

@peterp Did you experience this issue in your latest testing adventures? Should we just merge this? It's the easiest and quickest fix!

@peterp
Copy link
Contributor

peterp commented Nov 6, 2020

@RobertBroersma Right now I only have 1 test file for services so I had not experienced this, this is good to know though :)

@peterp
Copy link
Contributor

peterp commented Nov 6, 2020

I'm also not using yarn rw test api anymore, since I was getting an error about deleting tables, I'm doing cd api && yarn jest and using rawSQL that I introduce myself:

await db.$queryRaw`TRUNCATE TABLE "User" CASCADE;`

My gut feeling is that we shouldn't automatically delete data from the users test tables between tests.

@peterp peterp added this to the Next release milestone Nov 8, 2020
@peterp peterp merged commit 49784c0 into main Nov 10, 2020
@Tobbe Tobbe deleted the rc-jest-inband branch December 25, 2020 16:05
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.

5 participants