Allow configuring test database

[RobertBroersma]

👋 Hi again

I created a little setup that allows users to define a dev db and a test db in their redwood.toml file as discussed as an option in #502

I think it works pretty well.

I want to move (at least) one thing around: move the database up to inside the api test setup file. This would allow us to more easily split up jest project files later and separate concerns from the CLI command. (Also allowing users to pass jest --flags to yarn rw test.

I would also like to test this somehow. Maybe this could be part of some kind of test suite for the tutorial that was talked about during yesterday's call.

When this PR is a bit more polished it would also be great to test it out with some other databases. However until you can set provider through an env variable, that option won't really do anything. So it would currently be limited to being able to configure multiple of the same db provider.

Example redwood.toml: https://github.com/RobertBroersma/redwood-testing/blob/master/redwood.toml

Example service test file: https://github.com/RobertBroersma/redwood-testing/blob/master/api/src/services/posts/posts.test.js

@peterp @thedavidprice What do you think, does this look usable?

[thedavidprice]

@RobertBroersma I'm excited about the overall approach here. The posts.test.js example service test file looks 🤩 Even I would be able to use that!

I want to move (at least) one thing around: move the database up to inside the api test setup file. This would allow us to more easily split up jest project files later and separate concerns from the CLI command. (Also allowing users to pass jest --flags to yarn rw test.

^^ you're referring to the "test" db, correct? I wasn't quite sure I'm following this part -- but for me if you mean test.db would live inside api/prisma that makes sense. And I agree this is a priority --> "Also allowing users to pass jest --flags to yarn rw test."

DB Config and Command

Peter is removing the DB process from packages/cli/src/commands/dev.js in #699 It seems you were adding the Env Var... maybe do this with Webpack development config -- oh, wait, will that work for testing?

I'm not quite understanding the config options "test" and "dev" -- how are they different? Or is it that you're just making the existing "dev" option from env.default explicit in the config, and also adding the "test"?

I know Peter has been thinking about how to handle the Database in general as he's been doing work on Sides. I'll default to him in general, but some thoughts:

• the "dev" feels redundant in case of existing schema.prisma, could we read values from there if exists?
• wouldn't dev.provider and test.provider always be the same?
• what if you required by convention the two files, test.db and dev.db, to always be located in same directory by convention?

Misc

• reminder to add test.db to .gitignore
• there will be several doc updates when this one merges (new Test doc, app config doc...), be sure to help me remember to loop in Dom and we'll help to update.

[RobertBroersma]

@thedavidprice Thanks for the feedback! I'll try to explain my thoughts going into the points you mentioned.

^^ you're referring to the "test" db, correct? I wasn't quite sure I'm following this part -- but for me if you mean test.db would live inside api/prisma that makes sense. And I agree this is a priority --> "Also allowing users to pass jest --flags to yarn rw test."

I was more referring to how I'd like to remove calling yarn rw db up to (maybe) jest.setup.api.js file. Does that make sense? Not sure where else to put it.

the "dev" feels redundant in case of existing schema.prisma, could we read values from there if exists?

I was thinking it would be weird to define one DB from schema.prisma/env and the other from redwood.toml. "dev" would be for development, "test" would be for testing (the latter is cleared before every test, you wouldn't want that for your dev db!).

wouldn't dev.provider and test.provider always be the same?

Tbh I don't know! I could imagine having a Postgres instance for dev and a SQLite (inmemory) for test for speed. I know other's don't really like this approach tho.

what if you required by convention the two files, test.db and dev.db, to always be located in same directory by convention?
That could work for SQLite, but not for any other DB I guess?

[thedavidprice]

I was more referring to how I'd like to remove calling yarn rw db up to (maybe) jest.setup.api.js file. Does that make sense? Not sure where else to put it.

Ah, I completely misunderstood. That does make sense to me. Although, let it be clear that I defer all decisions and primary direction to @peterp 😆 And I do know there's some re-thinking happening about how to name structure the current "primsa/" to make it more flexible and general as "db/".

I'm thinking out loud, but re-reading your comments makes me question how much of this config should initially be exposed. A part of this feels to me like we could move forward under the "Redwood's handles API tests like this out of the box and makes these convention assumptions..." E.g. you're going to have a schema.prisma and we're going to handle where your test.db is and whether or not it uses SQLite.

Anyway, that's my day 2 take. Kicking things over to @peterp

[RobertBroersma]

I'm thinking out loud, but re-reading your comments makes me question how much of this config should initially be exposed. A part of this feels to me like we could move forward under the "Redwood's handles API tests like this out of the box and makes these convention assumptions..." E.g. you're going to have a schema.prisma and we're going to handle where your test.db is and whether or not it uses SQLite.

@thedavidprice Out of the box defaults sound good to me!

In fact, I think the way I set it up it should already default to this config 😅

[RobertBroersma]

@thedavidprice @peterp I'm also considering whether or not we should expose an Apollo Test Client

You would then instead of calling the services directly, test your services by sending queries. This involves the entire request chain, which would e.g. prevent typos in your sdl files! It's a more realistic representation of what the actual consumer of the API would do.

Downsides would include awkward location of the queries? I wouldn't want to duplicate the queries used in Cells just to test them in the API side. But I also don't want to import them from a Cell? Do I locate them somewhere else? 🤷‍♂️

This "problem" might be mitigated by the ability to do e2e tests in the web side (something I am working on!), which I would personally use most, and then I would only test on the API side things that aren't (directly) consumed by the frontend I guess.

Anyway I hope I'm making some kind of sense, rant over!

[peterp]

Hey @RobertBroersma 👋,

I'm going to summarise my understanding of the thing we're trying to solve: The main idea is that we're able to run tests against a real database.

The steps are the following:

1. The test command is run for the api side (where a db is active): yarn rw test api
2. We search for a connection string in the env-vars.
3. The database doesn't exist, since it should not exist between test run. (If it does, should we delete it?)
4. The database migrations are run and the tables are created. (We do not need to run them in sequence, we just need the tables.)
5. Prisma connects to the database.
6. Prisma does stuff to the database.
7. The tests end. The database is destroyed.

So, what do we need?

1. We need to know that you're running the api tests and that you're using a database.
2. If they're using a database we need to know the database connection string. .env.defaults, I think we should name this TEST_DATABASE_URL. If this is not set then we need to throw.

[peterp]

You would then instead of calling the services directly, test your services by sending queries.
[...]
This "problem" might be mitigated by the ability to do e2e tests in the web side

I think you're right that it probably makes more sense that this works with e2e tests! I think there's some benefit in having unit tests for services. I think our diagnostic stuff from decoupled studio will solve the majority of issues that people will have when integrating the SDL to the services side. (We should jump on a call and I can show you some of the diagnostics stuff, I think it's right up your alley of interesting things to work on.)

[RobertBroersma]

3. The database doesn't exist, since it should not exist between test run. (If it does, should we delete it?)

I think we should!

So, what do we need?

1. We need to know that you're running the api tests and that you're using a database.
2. If they're using a database we need to know the database connection string. .env.defaults, I think we should name this TEST_DATABASE_URL. If this is not set then we need to throw.

I think right now it could easily work using an env var for the connection string. The other thing we might need is a TEST_DATABASE_PROVIDER variable.

My initial worry was that e.g. a postgres db might have (in the future?) more config options than just a provider and a connection string. If this is not the case I think the env var route is indeed easier.

In any case I can simplify the PR so that it works with env vars instead of redwood.toml for starters!

[RobertBroersma]

You would then instead of calling the services directly, test your services by sending queries.
[...]
This "problem" might be mitigated by the ability to do e2e tests in the web side

I think you're right that it probably makes more sense that this works with e2e tests! I think there's some benefit in having unit tests for services. I think our diagnostic stuff from decoupled studio will solve the majority of issues that people will have when integrating the SDL to the services side. (We should jump on a call and I can show you some of the diagnostics stuff, I think it's right up your alley of interesting things to work on.)

Agreed! I guess the simplicity of testing services directly outweighs the benefits of running the queries.

I'd be very interested to see the diagnostics stuff. Aldo's presentation last week looked very promising, but I'd be lying if I said I understoof all of it. 😅

[peterp]

I think right now it could easily work using an env var for the connection string. The other thing we might need is a TEST_DATABASE_PROVIDER variable.

Ah, good call!

My initial worry was that e.g. a postgres db might have (in the future?) more config options than just a provider and a connection string. If this is not the case I think the env var route is indeed easier.

I can't really think of anything else off the top of my head right now, but I'm sure we can come up with something elegant if/ when that happens.

[RobertBroersma]

@peterp I've replaced the redwood.toml stuff by 1 env var for now (TEST_DATABASE_URL), I figured we can see about the provider when support for it lands :)

I keep getting this error tho when running the tests:

/mnt/c/Home/repos/testing/node_modules/.bin/jest --passWithNoTests --config ../node_modules/@redwoodjs/core/config/jest.config.api.js
  console.error
      plusX Execution permissions of /mnt/c/Home/repos/testing/node_modules/.prisma/client/query-engine-debian-openssl-1.1.x are fine +0ms

      at Function.createDebug [as log] (../node_modules/@prisma/debug/dist/index.js:66:1)
      at enabled (../node_modules/@prisma/client/app/src/node_modules/.pnpm/debug@4.1.1/node_modules/debug/src/common.js:119:1)
      at debug (../node_modules/@prisma/debug/dist/index.js:47:1)
      at Object.fixBinaryTargets [as plusX] (../node_modules/@prisma/engine-core/dist/util.js:21:1)
      at NodeEngine.flags [as getPrismaPath] (../node_modules/@prisma/engine-core/dist/NodeEngine.js:341:1)
      at ../node_modules/@prisma/engine-core/dist/NodeEngine.js:400:24

Can't really grasp what that means, any ideas?