jbrumwell / mock-knex

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

Support for Builder:modify() #122

Closed CodeSpent closed 3 years ago

CodeSpent commented 3 years ago

Proposing support for QueryBuilder:modify().

Currently when using mock-knex, if you are using .modify(), it will ignore this and fire the original unmodified query which makes this case impossible to test when using something like an implementation with Express for query params.

Example using modify() to alter the query if req.query.name exists.

await db("users")
    .modify((queryBuilder) => {
      if (req.query.name) {
        queryBuilder.where("name", "ILIKE", req.query.name);
      }
    })
    .then((results) => {
      res.json(results);
    })

This method is very valuable and I would not want to have to refactor code to having different queries in if statements or switch cases.

jbrumwell commented 3 years ago

@CodeSpent agreed, I will implement it but I can't give you a timeline at the moment. I would be open to PR on this :+1:

CodeSpent commented 3 years ago

Sounds good.

I am willing to take on a PR for this, but unfortunately I'm facing issue after issue getting this set up locally and without any contributing docs, its a little more of a derail than I anticipated.

Can potentially take a look if I have any revelations, but if it'd be quicker to add some more refined docs on getting started with contributing, that'd suffice, 'cause I can see how the actual implementation could work, but without being able to easily get this running locally, its tough to proceed.

jbrumwell commented 3 years ago

@CodeSpent Thank you for looking at it :) It has been awhile since I have done any work on this project but from memory;

terminal 1

# install dependencies
make install 
# build and watch for changes
make watch 

terminal 2

# run on currently install knex
make test

# run on all supported knex
make test-suite

Let me know if you run into any issues, my apologies for lack of contribution docs

felixmosh commented 3 years ago

Try knex-mock-client it is not monkey patching knex, but working with a different approach.