search

jbrumwell / mock-knex

A mock knex adapter for simulating a database during testing
MIT License
239 stars 71 forks source link

Add support for knex 16 #84

Closed CodisRedding closed 5 years ago

andyclarke commented 5 years ago

This looks like it's just what I was looking for, thanks

jbrumwell commented 5 years ago

@CodisRedding thanks for putting this together, looks we have some failing tests here that look to be related to this code change;

https://github.com/tgriesser/knex/commit/1eac063b28baf25b5e3140bd71c6358d056f1695#diff-3fd14dc9565c83e98abde069b4a73196R89

jbrumwell commented 5 years ago

Okay so this seems to be a bug in bookshelf, they pass the bookshelf context instead of knex context to the knex method.

https://github.com/bookshelf/bookshelf/pull/1926

According to https://github.com/bookshelf/bookshelf/pull/1872 they don't plan on keeping up with knex until after they drop node v4 support. IMO we can drop the bookshelf tests or wait until bookshelf supports knex latest. I'm leaning toward dropping bookshelf support as this is mainly about mocking knex itself.

jacobriddle commented 5 years ago

If that is the case, I agree with dropping the bookshelf tests. Bookshelf 1.0 still seems to be a ways out and multiple updates to knex have been released since that issue was reported. I suspect that trend may continue!

To add fuel to the fire, our current use case has us using mock-knex for mocking knex itself without bookshelf.

CodisRedding commented 5 years ago

Hey @jbrumwell, I noticed that they were failing before I made any changes so I figured it would come up in discussion here.

CodisRedding commented 5 years ago

@jacobriddle I also use this without bookshelf.

jbrumwell commented 5 years ago

@CodisRedding Thank you for your contribution :) Merged and published in 0.4.3 :clinking_glasses: