felixmosh/knex-mock-client

Returning not captured by tracker

Closed this issue ยท 28 comments

I have an insert query with returning('*').
Insert is captured in tracker.on.any but not returning.
Any suggestions ?

Can you share the code & the test?

await ModelName.insert(params) .returning('*') .first()

Another case
I tested a query involving select & graph fetch, only select was captured.
await ModelName.query() .select('abc') .withGraphFetch('relationName')

Only select was captured.
Select of graph fetch is not captured.

I am using objection-js so it internally uses knex.

How should I do it ?

I'm not familiar with pg so much, but let me investigate it.
Did you passed the dialect property?

withGraphFetch is not a knex option, so I don't know what objection generated behind the hood.

I've tried to run insert with returning,

await db('table_name')
        .insert({ id: 1234, created_at: '2022-05-10', updated_at: '2022-05-10' })
        .returning('*');

Printed out the call history, and I do see insert catch with the returning part.

{
  method: 'insert',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [Array],
  __knexQueryUid: 'BEJ_r5crW6qLctx8BmrVY',
  sql: 'insert into "table_name" ("created_at", "id", "updated_at") values (?, ?, ?) returning *',
  returning: '*',
  queryContext: undefined,
}

You can write something like this to catch it

tracker.on
  .insert(({ sql }) =>
    ['insert into', 'table_name', 'returning *'].every((part) => sql.includes(part))
  )
  .responseOnce({ id: 1234, name: 'foo' });

So what is the issue?

This returning you have used is objection-js library's ?

Just read this #20 (comment).
Objection generates a new query for graph fetch.
Just to give the working flow, in mock-knex graph fetch queries are captured/tracked as separate query (different step number).

I don't know but for some reason, graph fetch queries (basically extra select queries) are not captured by this mock client. They are ultimately called to the same client.

Will investigate some more & update.

Sounds like you are not returning the data that objection requires for its graph fetch... but it is just a guess.
Can you prepare a small repo with Objection & knex-mock-client?

Perfect guess!
I also found that I was not returning the join field value that is needed for graph fetch query.
When I added that, it worked!
Thanks.

Can we close this issue?

Also, the main issue was for returning.

How I used it was await ModelName.query().insert(params) .returning('*') .first(). This is in objection.
Above worked fine. I can now see returning in generated sql.

But if I add transaction to objection query, insert is captured but returning is missing in generated sql.
See below snip.

const knexRead = await knex({
  // for mocking values
  client: MockClient,
  dialect: 'pg',
});
const trx = await knexRead.transaction();
// the transaction object returned from above is passed to below query method
await ModelName.query(trx).insert(params).returning('*').first();

then at some place

await trx.commit()

In actual code (real interaction with db), returning works & returns data but in unit-test mocking, returning does not appear in generated sql.

I feel I might be doing something wrong here. Is this the intended way of generating transaction for mock as well ?

There are test with transactions, which works with nested syntax

await db.transaction(async (trx) => {
  await db('table_name').insert({ name: faker.name.firstName() }).transacting(trx);
  await db('table_name').delete().where({ name: faker.name.firstName() }).transacting(trx);
});

Maybe there is an issue with the commit style, I'll check it.

Okay sure!
Just let me know that.
If it does not work commit style, I may do unit-test without transaction.

I've extracted all transaction tests to a separate spec, and added one with commit style.
https://github.com/felixmosh/knex-mock-client/blob/master/tests/transaction.spec.ts
It works as expected. (actually, the mock ignores transactions by empty implementation so your issue probably not related to transactions)

Thanks for the effort.
Did you check if the insert incase of transaction has returning attached to generated sql ?
I see in the test that, it is expecting one insert call which is happening with me as well.
Generated sql does not have returning to it.

The returning part is related to dialect property, can you check that you are passing it?

It looks like a bug in the this lib, I'll investigate asap.

The returning part is related to dialect property, can you check that you are passing it?

I am passing pg dialect.

Looks like it is much harder than I thought, from some reason, knex for transactions is instantiating the MockClient using Object.create which skips the constructor :|

I've an idea how to fix this, but i need to invest sometime in the solution.

No problem!
Meanwhile I would unit-tests without transaction.

The fix released in v1.8.2

Thank you for reporting this issue ๐Ÿ™๐Ÿผ

Tested! Working good.

I am getting this message, not an issue though.
(node:62616) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 start listeners added to [MockClient]. Use emitter.setMaxListeners() to increase limit

If this is of any concern, I can create a new issue. If not let it be.
Is this due to multiple async unit-tests service function running in parallel by jest ?

Can you open a new issue with this, I'll check it.

I am getting this message, not an issue though. (node:62616) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 start listeners added to [MockClient]. Use emitter.setMaxListeners() to increase limit

If this is of any concern, I can create a new issue. If not let it be. Is this due to multiple async unit-tests service function running in parallel by jest ?

MockClient.EventEmitter.defaultMaxListeners = 100

@colvint can you open a new issue with details of how to reproduce this error?

It's rather involved to repro. Given the warning, I presume if you add enough trackers in just enough beforeEachs, you'll trigger it. Above, I'm pointing out one can silence the warnings by upping the number of event emitter listeners.