5

I have a python function which if passed bad data gets caught in an infinite loop. I'd like to write a unit test to confirm that it handles bad parameters gracefully. The problem is of course that if it doesn't detect the bad parameters it won't return.

Is it acceptable to use threads to write a test for this type of thing?

import threading, unittest
import mymodule

class CallSuspectFunctionThread(threading.Thread):
    def __init__(self, func, *args):
        self.func = func
        self.args = args
        super(CallSuspectFunctionThread, self).__init__()
    def run(self):
        self.func(*self.args)

class TestNoInfiniteLoop(unittest.TestCase):
    def test_no_infinite_loop(self):
        myobj = mymodule.MyObject()
        bad_args = (-1,0)
        test_thread = CallSuspectFunctionThread(mybj.func_to_test, *bad_args)
        test_thread.daemon = True
        test_thread.start()
        # fail if func doesn't return after 8 seconds
        for i in range(32):
            test_thread.join(0.25)
            if not test_thread.is_alive():
                return
        self.fail("function doesn't return")

The only problem I see with this is that if the test fails I'm stuck with this additional thread, potentially consuming cpu and memory resources, while the remainder of the tests are executed. On the other hand, I've fixed the code and the likelihood of regression here is very slim, so I don't know if it even matters whether I include the test or not.

Aryeh Leib Taurog
  • 5,370
  • 1
  • 42
  • 49

6 Answers6

12

You can add a timeout decorator. It's good to separate the logic of your testcase from the timeout mechanism implementation. This will make your code more readable and easier to maintain.

See http://pypi.python.org/pypi/timeout .

Michał Šrajer
  • 30,364
  • 7
  • 62
  • 85
  • 1
    +1 for simplicity. I like the fact that the library uses SIGALRM, which seems more appropriate than threads. I note that it doesn't seem to work under windows. This doesn't directly address the question though, which is the advisability of such a test. – Aryeh Leib Taurog Apr 14 '11 at 17:12
  • 1
    -1: Won't work very well in general. One day you have a busier-than-normal machine and it takes longer than the timeout and you're debugging the overall system workload, not your application. You can't do this with a test. – S.Lott Apr 14 '11 at 17:31
  • 4
    @S.Lott: The end user doesn't care why your program hung up - it should handle all cases as gracefully as possible, including those where it has inadequate resources. It's simply unacceptable for an application with a GUI to take over a second to provide feedback to user actions, for example. You needn't set a timeout that's anywhere near how long a function should take to return. If a function normally takes 0.01 second to return, it's fine to set it at 10 seconds if the function can take that long and be considered acceptable. – ArtOfWarfare Aug 20 '13 at 15:22
1

I would add the test to test for the infinite loop, since if it hangs during the tests then at least you've caught the bug before production. If the chance of it happening is slim (as you say), then i wouldn't worry about a complicated test, but I'd test the code nonetheless.

Gevious
  • 3,214
  • 3
  • 21
  • 42
0

Maybe in your case you can solve it as the accepted answer say. Good for you. However there are cases (for integration and smoke tests, not unit tests) when you can't rule the "hangs forever" case. For these cases, I like this timeout solution.

Community
  • 1
  • 1
Davide
  • 17,098
  • 11
  • 52
  • 68
0

Having a test for this condition is a good idea. When code is tested, you can be more confident about it's quality.

As for the thread you create to test the function, here are two ways to kill a thread in python. Since you are inside a function that has the infinite loop, you will have to use the second method to break out of the loop.

Community
  • 1
  • 1
unholysampler
  • 17,141
  • 7
  • 47
  • 64
  • I saw that, but it looked like overkill--pardon the pun. If I keep the test despite S. Lott's advice against it, I'll probably use the timeout decorator, as Michał Šrajer suggested. – Aryeh Leib Taurog Apr 14 '11 at 17:35
0

Actually you don't have to catch it. As long as it is in your tests, you will surely notice if it goes wrong.

Thomas Ahle
  • 30,774
  • 21
  • 92
  • 114
-1

You can't possibly test for this. What if the other thread is just "slow" or worse "livelocked" (i.e., waiting for a shared resource)? Any assumption you make can run afoul of reality.

You can't shrug and say "It always used to terminate in 1.5 seconds, but now it seems to take a little longer. I'll try 2 seconds, and see if it passes." That's just not acceptable.

You have to prove termination of all loops all the time. All loops.

Anything less is simply a design that is so poor that it should never see the light of day.

If you can't prevent and infinite loop, you're not done designing.

It's not a thing that can be tested for.

S.Lott
  • 384,516
  • 81
  • 508
  • 779
  • Good points. So this is a limitation of unit testing. How does one **prove** termination though? In my case, I'm trying to determine some simple properties of a mathematical function. The edge cases seem to be where the curve described isn't well behaved (i.e. doesn't converge/no local extreme). I admit I could be taking the lazy approach; it is easier to write a test and move on then figure out where it's getting stuck. – Aryeh Leib Taurog Apr 14 '11 at 17:23
  • 1
    @Aryeh Leib Taurog: This is not a limitation of "Unit Testing". This is a limitation of all testing. You cannot test for termination. You must prove it. If you're doing analysis of functions, you have to actually do the math to determine what constraints define convergence and reject all initial conditions that can't converge. Sorry, but you have to do the math. – S.Lott Apr 14 '11 at 17:29
  • 3
    Not all problems have a closed solution. Determining a complete set of constraints on the input in this case is not a realistic approach. What I meant was to simply use a loop counter to halt calculation once it's gone too far. But there also it's not always simple to know how far is too far. – Aryeh Leib Taurog Apr 14 '11 at 19:21
  • @Aryeh Leib Taurog: A loop counter can be used to **prove** termination. Use it. That's what the "Numerical Recipes" book suggests for problems like this. If you can't work out some kind of loop limit, you have difficult math to do. But you must do it. A "timeout" unit test cannot be made to work. A loop counter is what you **must** do. Setting the upper limit isn't easy, but it's at least sensible. – S.Lott Apr 14 '11 at 19:25
  • I agree in theory that this approach could lead to false fails but in practice I think that's unlikely in most cases. In my experience tests are usually run either on the programmer's desktop or a dedicated build machine. It isn't difficult to get fairly consistent reproducible conditions. Also I think a false fail is probably preferable--especially for critical code--than no test which is essentially a false pass. – Aryeh Leib Taurog Apr 14 '11 at 19:36
  • @Aryeh Leib Taurog: A loop which cannot be **proven** to terminate is simply wrong. Sorry. It's not a *theoretical* position. It's a simple matter of definition. You cannot test a loop which cannot be proven to terminate. There are too many confounding factors that make it (a) technically wrong, and (b) impractical. A loop counter is trivial to implement and **proves** termination. Just add the loop counter. Please. – S.Lott Apr 14 '11 at 19:38
  • @Aryeh Leib Taurog: Think for a moment. You can't add a loop counter because you don't know how long it will take. For the **exact same reason** you can't use a timeout. Don't use complicated multi-threaded testing. Just use a loop counter. – S.Lott Apr 14 '11 at 19:39
  • I'm accepting this answer because it most directly addresses the issue posed. I agree that the loop counter is necessary and recognize that normal testing methods won't cleanly cover that loop counter code. However, Michał Šrajer's non-thread solution is elegant and simple on systems supporting SIGALRM, so I am not completely convinced (yet) that there is no value in implementing such a test as well. – Aryeh Leib Taurog Apr 14 '11 at 20:49
  • @Aryeh Leib Taurog: It's not a testable feature. By definition. There's a logical contradiction in attempting to test for this. Please research the "Halting Problem". It's not *theoretical* in this case. It's intensely practical. It requires proof. And mathematical analysis. – S.Lott Apr 14 '11 at 21:16
  • @S.Lott: The entire point of unittests is to test attributes of your function which should be true. One attribute most functions implicitly have is that they should return promptly. – ArtOfWarfare Aug 20 '13 at 15:18
  • I think this come down to something like mathematical theory vs real world. Yah maybe your machine will be slow...but 10 seconds slow!!!! Either way maybe you get a bonus of testing that your function is slow as heck now. Not everything has to be deterministic to be useful and practical. The point most of the time is to prevent embarrassment of loss of $$$ not to have perfection in your build failures. If lifes are on the line...find a better way. If tweets are on the line practicality wins. – Eric Twilegar Apr 30 '14 at 18:32