kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
10.8k stars 275 forks source link

Writing Unit Test #801

Closed vnijat93 closed 11 months ago

vnijat93 commented 11 months ago

This is NOT an issue but more of crying for help

We are using Kysely for an enterprise API using AWS Lambda and AWS Aurora (Postgres). One of the requirement is having unit test coverage of at least 80%.

Kysely is AWESOME, but i have been struggling with writing unit tests. (I'm new to Typescript btw). These unit test will run on jenkins, so no Postgres database for us to connect to. I figured that I might need to mock certain calls but don't know how.

One of my functions look like this:

const dialect = new PostgresDialect({ pool: new Pool(databaseConfig)})
const DB = new Kysely<Database>({ dialect })
const results = await DB.withSchema("database_schema")
    .selectFrom("table")
    .where('someId', '=', queryParamSomeId)
    .where('anotherId', '=', queryParamAnotherId)
    .selectAll()
    .execute()

Does anyone have any example that I can learn from?

igalklebanov commented 11 months ago

Hey 👋

Ideally, you'd want to do integration tests against real engines - by spinning up a docker image. See Kysely's node.js tests for reference. Alternatively, use an in-memory mock (e.g. https://github.com/oguimbal/pg-mem - requires implementing a custom kysely dialect for now) or SQLite (different engine, less ideal) for tests.

Don't try to mock builder pattern APIs. If anything, wrap the usage and mock the wrapper.

francescovenica commented 9 months ago

Given something like:

export const db = new Kysely<DB>({
  dialect: new PostgresDialect({  pool: new Pool()  }),
  plugins: [new CamelCasePlugin()],
});

export const fetchAccount = () => db.selectFrom("account").executeTakeFirst()

I'm able to unit test the function and check that the query is correct

const mocks = vi.hoisted(() => ({ query: vi.fn() }));

vi.mock("@/db", () => ({
  __esModule: true,
  ownerDb: new Kysely<DB>({
    dialect: new PostgresClientDialect({  client: mocks  }),
    plugins: [new CamelCasePlugin()],
  }),
}));

it("query", async () => {
    mocks.query.mockResolvedValue({ rows: [{ .... }] });
    await fetchAccount();
    expect(mocks.query.mock.calls).toMatchInlineSnapshot("here you will see the query");
  });
thelinuxlich commented 9 months ago

But is it useful?

francescovenica commented 9 months ago

as unit test I would say yes....if I have a function that create a user and a subscription, something like:

export const createAccount = async () => {
  await db.insert("account").executeTakeFirst() 
  await db.insert("subscription").executeTakeFirst()
}

I want to be sure that calling this createAccount() will run 2 particular queries....

Obviously for integrations test we shouldn't mock the connection and just connect to a real db (like wrote in first comment)

thelinuxlich commented 9 months ago

Then why unit test if the integration test will cover the same field and more?

francescovenica commented 9 months ago

mmm maybe it is just a limitation for my experience...but usually I do unit test to be sure that a piece of code does what is meant to do and integration test to test the whole flow....otherwise what would be the point of unit test? but again, I never built myself an app with millions of users, I only worked for companies that do tests in this way, so I might be wrong :)

thelinuxlich commented 9 months ago

But what value do you gain from asserting something that will always happen(at least on the happy path, as createAccount isn't a pure function)? At least the integration test asserts the expected data transformation.

francescovenica commented 9 months ago

I think the main point is to mock the client so you can run unit tests without having a db running, maybe the assertion can be skipped if you have integration tests, but if I think about a function like this:

const registerBusinsess = ({ db }: { db: Kysely<DB> }) => { 
  const account = await db.insert("account").executeTakeFirst() 
  const stripeSubs = await stripe.subs.create({ meta: { account: account.uuid }});
  const subscription = await db.insert("subscription").set({ stripeSubs: stripeSubs.id }).executeTakeFirst();
  await sqs.sendMessage({ event: "new-subscription", data: subscription })
}

I would like to have a test like:

it("register a businsess", () => {
   await registerBusinsess({ db });
   expect(dbMock.query).toHaveBeenCalledWith("...");
   expect(stripeSubs.create).toHaveBeenCalledWith("...");
   expect(dbMock.query).toHaveBeenCalledWith("...");
   expect(sqsMock.sendMessage).toHaveBeenCalledWith("..);
})
thelinuxlich commented 9 months ago

That test doesn't assert anything that wouldn't happen anyway