Mock-knex 0.3.3 failed on save and update operations

[Getz85]

My unit tests worked fine with Mock-knex 0.3.2. With the last version, I've got this error when I want to test some insert or update operations inside a transaction:

On insert:

Unhandled rejection TypeError: Cannot read property 'insertId' of undefined
    at Client.processResponse (C:\Projets\Front\webapp-internet\node_modules\knex\lib\dialects\mysql\index.js:116:21)
    at C:\Projets\Front\webapp-internet\node_modules\knex\lib\runner.js:116:28
    at tryCatcher (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\util.js:26:23)
    at Promise._settlePromiseFromHandler (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\promise.js:507:31)
    at Promise._settlePromiseAt (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\promise.js:581:18)
    at Async._drainQueue (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\async.js:128:12)
    at Async._drainQueues (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\async.js:133:10)
    at Immediate.Async.drainQueues [as _onImmediate] (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\async.js:15:14)
    at processImmediate [as _immediateCallback] (timers.js:367:17)

On update:

Unhandled rejection TypeError: Cannot read property 'affectedRows' of undefined
    at Client.processResponse (C:\Projets\Front\webapp-internet\node_modules\knex\lib\dialects\mysql\index.js:120:20)
    at C:\Projets\Front\webapp-internet\node_modules\knex\lib\runner.js:116:28
    at tryCatcher (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\util.js:26:23)
    at Promise._settlePromiseFromHandler (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\promise.js:507:31)
    at Promise._settlePromiseAt (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\promise.js:581:18)
    at Async._drainQueue (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\async.js:128:12)
    at Async._drainQueues (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\async.js:133:10)
    at Immediate.Async.drainQueues [as _onImmediate] (C:\Projets\Front\webapp-internet\node_modules\knex\node_modules\bluebird\js\main\async.js:15:14)
    at processImmediate [as _immediateCallback] (timers.js:367:17)

This is an exemple of code which failed:

bookshelf.transaction(function(t) {
  return Customer.forge(customer)
    .save(null, {
       method: 'insert',
       transacting: t
    }).then(function(insertedCustomer) {                    
        return insertedCustomer.customers().attach(customers, {
           transacting: t
        });
     }).then(function(result) {
          resolve(customer);
     }).catch(function(err) {
           reject(err);
     });
 });

And this is my tracker code:

var customer = [];
var insertedCustomer = {
   id: "TEST",
   name: "TEST"
  };
var customers = ['2', '3', '4'];
tracker.on('query', function sendResult(query) {
  if (query.method === 'select') {
    query.response(customer);
  } else if (query.method === 'insert') {
      if (query.bindings.length === 3) {
         customer.push(insertedCustomer);
         customer[0].customers = [];
       } else {
          customer[0].customers.push(query.bindings[0]);
       }
      query.response(customer);
  } else {
       query.response(customer);
  }
});

[jbrumwell]

@Getz85 What version of knex / bookshelf are you using?

[dustinmartin]

@Getz85 Does the test work once you remove the transaction? I might be seeing a similar issue.

[Getz85]

@jbrumwell knex version 0.11.10 bookshelf version 0.10.0

@dustinmartin Indeed, the test work fine if I remove the transaction!

[dustinmartin]

@Getz85 Awesome...so we might have the culprit. Checkout my comment in issue #40. Perhaps it details some similarities in your setup. I'm curious to know when you mock Knex and what DB you are using or if there is anything else that sticks out. Thanks!

[Getz85]

@dustinmartin I use MySQL, so we've got another use case here ;) I mock knex before passing it to bookshelf, like in the tests apparently.

  before(function(done) {
    db = knex({
      client: 'mysql',
      useNullAsDefault: true
    });
    mockKnex.mock(db);
    bookshelf = require('bookshelf')(db);
    bookshelf.plugin('registry');
    var Customer = require('../../models/customer')(bookshelf);
    done();
  });

[jbrumwell]

I have added a branch here with a transaction test inside of bookshelf, https://github.com/colonyamerican/mock-knex/tree/bug/bookshelf-transactions, which passes but this could be client specific. If I get time today I'll try and update the code to run through a mysql / postgres client and see if we can track it down

[dustinmartin]

@jbrumwell Thanks so much for looking into this. Let me know if I can provide any information or test something for you.

[dustinmartin]

@jbrumwell Any luck reproducing the bug with mysql or postgres?

[jbrumwell]

Took a little longer then expected, can you guys give this branch a test?

https://github.com/colonyamerican/mock-knex/tree/feature/test-databases

[jbrumwell]

Released version 0.3.4, let me know if it resolves the issues

[dustinmartin]

@jbrumwell That worked perfectly! Thanks! What ended up being the problem?

[jbrumwell]

There were a few issues

• Transactions use the constructor's prototype from the client here, https://github.com/tgriesser/knex/blob/master/src/transaction.js#L188, which we were not mocking. This was resolved by adding it to our spec https://github.com/colonyamerican/mock-knex/commit/da037a0920abbdbd6cb598b729c22783c6c6d3da#diff-15098551bfebe893b49d9701540e63eaR92

This resolved the issue with transactions

• The above also caused a side effect in the mocking algo, since we were mocking both the instance and the prototype it didn't have a reference to the original method to revert. This was resolved by mocking the constructor/prototype methods after the instance to ensure we got a reference to the original function. https://github.com/colonyamerican/mock-knex/commit/da037a0920abbdbd6cb598b729c22783c6c6d3da#diff-834d7a420929f7d15c58e33126f4cc96R54
• Some of the stream tests were failing, which was resolved by overwriting the stream method https://github.com/colonyamerican/mock-knex/commit/da037a0920abbdbd6cb598b729c22783c6c6d3da#diff-afb57afee1821745f7d0c2b15087aec2R49
• Another issue was there was a property collision in the query, postgres uses response in their query object. To avoid these issues we nest our properties now, https://github.com/colonyamerican/mock-knex/commit/da037a0920abbdbd6cb598b729c22783c6c6d3da#diff-d8494ca7e744216ae18ca4aa7d9eab7cR18

Thanks for your help :)

[dustinmartin]

Fantastic! Thanks again for resolve this. Happy to help!