4

i am having a simple app example here:

say i have this piece of code which handles requests from user to get a list of books stored in a database.

from .handlers import all_books
from flask import jsonify

@apps.route('/show/all', methods=['GET'])
@jwt_required
def show_books():
    user_name = get_jwt_identity()['user_name']
    books = all_books(user_name=user_name)
    return jsonify(books), 200

and in handlers.py i have :

def all_books(user_name):
        db = get_db('books')
        books = []
        for book in db.books.find({'read_by':user_name}):
            books.append(book)
        return books

but while writing unit tests i realised if i use get_db() inside all_books() it would be harder to unit test the method. so i thought this would be the good way.

from .handlers import all_books

@apps.route('/show/all', methods=['GET'])
@jwt_required
def show_books():
    user_name = get_jwt_identity()['user_name']
    db = get_db('books')
    collection = db.books
    all_books(collection=collection,user_name=user_name)

def all_books(collection,user_name):
        books = []
        for book in collection.find({'read_by':user_name}):
            books.append(book)
        return books

i want to know what is the good design to use? have all code doing one thing at one place like the first example or the second example is good.

To me first one seems more clear as it has all related logic at one place. but its easier to pass a fake collection in second case to unit test it.

  • @DocBrown return value of all_books is sent back to the user making the api request (edited above) & user_name is used as a filter in db.books.find({'read_by':user_name}) – anekix Dec 19 '18 at 13:47
  • Looks better now. – Doc Brown Dec 19 '18 at 17:31
3

When you split the db query from the filter logic like this ...

def all_books(user_name):
        db = get_db('books')
        return filter_books_by_user(db.books,user_name)

def filter_books_by_user(collection,user_name):
        books = []
        for book in collection.find({'read_by':user_name}):
            books.append(book)
        return books

... you get both - a function filter_books_by_user which contains the "logic", but no database access, and a function all_books which encapsulates the db query together with the filter logic. filter_books_by_user can be unit tested without the DB, all_books however is probably better validated by an integration test (including the DB).

1

The thing is that there is IO part (query a database, http request, local disk read/write operations, etc.) and there is post-processing logic: filtering, sorting, validation. There is little to no sense to test IO through the unit tests: integration tests were designed for this purpose and yet they proof that database or whatever else IO subject you're dealing with is there and replies as expected. However, it definitely does not mean that you should not separate your pure logic from an IO: that is what can be easily testable (as long as your database is mocked). So split it and cover pure part with unit tests, assuming that the IO part will work fine.

0

It's understandable that you want to unit test everything, although it's not a good idea. The all_books function does not contain any business related logic, it's just one of the hundred ways to get all books in the database.

That should be tested with integration testing, showing the books are properly returned using the database.

You could unit test this function, but that would mean making a mock object of the database that you'd inject and then you're only left with testing a for in. And unless you plan on changing your databases in the future (which never happens), that's ineffective and worthless.

  • Yeah. BTW, you know why "it never happens"? Because people hardcode to a particular database and it takes way too much efforts to get rid of that :) So, -1. Filtering (pure) part must be kept away from the IO. – Sereja Bogolubov Dec 20 '18 at 9:39
  • Did you really work on big projects and thought you’d change your whole database architecture all of a sudden and it wasn’t expected at all ? Seriously ? Abstract to be flexible against changes which are kinda expected, not against all possible changes. And by the way there was no filtering in the original post, simply a select * request. – Steve Chamaillard Dec 20 '18 at 12:19
  • Well, yeah, once I had really big project which wanted to move towards the graph-driven DB. And couldn't due to reason I've mentioned above. IMO databse fear arises from the leak of IO/Data persistence abstraction layer.Quite often existing kinda repositories are too tight to the SQL style, you know, and when you wanna get rid of SQL, even though repositories do exist, you can not. – Sereja Bogolubov Dec 20 '18 at 12:25

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.