Reputation: 23
I'm using Python, if that's relevant.
As I understand it, a unit test is a test of the smallest portion of code that is relevant to a single use case. Often times this is a single function or method.
So when I'm testing my function, I do not want to test any functions that it's calling. When writing tests, that's reasonably easy to do: Just concern yourself with the logic that your function is doing.
But when I have the following situation:
def is_prime(number):
if number <= 1:
return False
for element in range(2, number):
if number % element == 0:
return False
return True
def extract_primes(lst):
return [item for item in list if is_prime(item)]
I'm confused about how a test for extract_primes
would look like.
def test_extract_primes():
lst = [0, 1, 2]
result = extract_primes_from_list(lst)
assert result == [2]
# and some more similar tests for empty lists, lists with only primes
# and lists with no primes
But now I'm just implicitly testing whether is_prime
is doing its work correctly. And when is_prime
has a bug, it will also cause the test for extract_primes
to fail, which beats the point of unit tests, where you want to quickly spot the single point of failure.
So it seems to me, that I shouldn't be calling is_prime
in the first place. So I end up with something like this:
@unittest.mock.patch("path.to.module.is_prime")
def test_extract_primes(mock_is_prime):
mock_is_prime.return_value = True
lst = list(range(10))
result = extract_primes(lst)
assert result == lst
for i in lst:
assert mock_is_prime.called_with(i) # not sure if correct method name
But this is tiresome and inflexible at best. I cannot cause mock_is_prime
to return different values inside of the same test, unless I create a construct like:
bool_list = [True, False, True]
mock_is_prime.side_effect = lambda x: next(bool_list)
But that gets even more verbose. And as the number of function calls within the function under test increases, so does the stupid amount of boilerplate code to patch away those functions.
When I try to look for answers on the internet, I mostly get Java/C# dependency injection instructions, where they tell you to pass the correct needed object as parameter, and just create a dummy of that object for the method you're testing. Or I get debates about whether or not to test private methods. And that's fine, I understand those things. But I can't for the life of me figure out what to do with functions that depend on other function calls. Should I simply inject those functions?
def extract_primes(lst, is_prime_function=is_prime):
return [item for item in list if is_prime_function(item)]
And pass a dummy is_prime
function into extract_primes
under test? That just looks stupid and litters the function signature with weird parameters. And it's practically no different from patching the function away in the amount of work required. It just removes a single @patch
statement from the test.
So should I not be patching away functions in the first place? And if not, at what point does a function become worth patching away anyway? Only when it's manipulating the system? Or also when it's from a completely separate module?
I'm a little lost.
Upvotes: 1
Views: 599
Reputation: 48824
Unfortunately, there's no easy answer, short of "write really well structured code", which of course, is hard. I'll try to help shed some light on the subject by going through your question piece by piece.
As I understand it, a unit test is a test of the smallest portion of code that is relevant to a single use case. Often times this is a single function or method.
This is broadly correct, but the primary goal is to test the interface of that code; that is to say, given certain inputs, do you get the expected outputs. You don't want to care about how that block of code does what you want (if you do, you start writing change detector tests).
I do not want to test any functions that [my function is] calling.
Not quite; you don't want to care about what functions the code being tested calls. As far as your test is concerned, you don't care if it is a hard-coded mapping of inputs to outputs, or if it goes out and does a Google search* and parses the first result, as long as the behavior is what you expect.
Notice that your is_prime
function depends on the behavior of other "functions" too - <=
, range()
, %
, and ==
all have to behave a certain way for your is_prime
function to work correctly. That isn't a bad thing! You can be confident the functionality is_prime
depends on is working correctly because Python has already tested it. Whether they're tested at the same time or not doesn't really matter.
I'm just implicitly testing whether
is_prime
is doing its work correctly.
Not really. There's nothing about extract_primes()
that requires it use is_prime()
, in fact you might very well discover a better algorithm for this behavior later and want to swap it out. You want to test these two functions separately because otherwise your tests only work on the assumption that extract_primes()
calls is_prime()
.
If there is some reason in your actual use case that extract_primes()
must rely on is_prime()
, then you are likely over-coupling your code. Consider, as others have suggested, separating your predicate-like behavior from your filter-like behavior, so that the filtering works on any predicate.
when
is_prime
has a bug, it will also cause the test forextract_primes
to fail, which beats the point of unit tests, where you want to quickly spot the single point of failure.
Very true, and this is generally something you want to try to avoid. But all code is built upon other code, and it's impossible to always have one and only one test fail. If something deep in your code breaks, everything that depends on it will break too. One solution is to have layers of tests; unit and integration tests at a minimum, but you can also have layers inside of that, e.g. test your utility code first, and only test dependant code once those tests all pass.
[Injecting functions] is tiresome and inflexible at best.
Definitely. Don't do that (normally). In fact, a pretty good sniff-test of your code is "Do I have to do strange, cumbersome things with my code in order to write good tests?" If so, your design probably merits revisiting.
Of course, this isn't always possible. You've seen mocking, stubbing, monkey patching and dependency injection, all of which are tools for dealing with difficult-to-test code, but these are all tools for when you can't reliably just test the interface.
In other words, first try to write clean, loosely coupled code. Then, try to test the interface of each component in isolation. When you can't reliably do that, start looking at mocking and dependency injection and so on.
*You certainly might care about this from a performance or test stability perspective, but that's a separate issue. As far as that individual test is concerned, it doesn't care.
Upvotes: 1
Reputation: 47790
Generally speaking: It depends entirely on your design, but your initial attempt seems reasonable.
Assuming is_prime
and extract_primes
are both part of your class / module's public interface, they should both be tested. At that point, you want to test extract_primes
as a black-box, i.e. just ensuring that it fulfills its contract: give it a list and it returns the primes. The fact that it's using is_prime
internally is an implementation detail your unit test code shouldn't concern itself with.
The point of unit tests isn't necessarily that ONLY the earliest failure should trigger (of course, it's great if you can manage that); it's quite possible that a lot of dependent things fail if you break something high enough upstream. If is_prime
is broken, then it's perfectly valid to see a test failure for extract_primes
too, but just looking at the list of test failures is enough to immediately identify the root cause.
Also, this way you can do things like: try out other prime-testing functions, replace the list comprehension with a generator expression, or other refactors etc. under the hood without needing to change your test code.
Mocking should be reserved for external dependencies; patching every call that any function makes is ridiculously verbose, ties test code to implementation far too tightly, and doesn't really get you anything.
Upvotes: 2
Reputation: 7616
In general it is a good rule of thumb that when you reach a point where the best practices don't work for you then the problem occurred much earlier than that point. In your case you don't know what to do because you have two public methods calling one another. This is usually indicative of the fact that they don't belong in the same class. For instance a function that filters a list based on a predicate doesn't really have anything to do with prime numbers. This shouldn't be functionality provided by this class. If this is something that you are doing very often then you can create a list utils class that has a filter method. I think that testing the two methods, is_prime
and filter_list
, should be very easy.
In general classes should be designed to provide a set of functionality to a user. A class shouldn't be it's own user. That creates many problems not the least of which is the problem you encountered. Of course there are exceptions to this rule and the best design should be judged on a case by case basis but I find this to be a good rule of thumb.
Upvotes: 1