Skip to content

return statement is not stated as covered despite executed and asserted by tests #297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nedbat opened this issue Apr 9, 2014 · 9 comments
Labels
bug Something isn't working

Comments

@nedbat
Copy link
Owner

nedbat commented Apr 9, 2014

Originally reported by lc3t35 (Bitbucket: lc3t35, GitHub: lc3t35)


Here is the code

#!python
try:
      _id = foo.save(something)
      if _id is not None:
          return Result(RESULT_OK)
except OperationFailure:
      # this return statement is not stated as covered despite executed and asserted by tests
      return Result(RESULT_ERROR, MESSAGE)

Here is the test :

#!python
from mock import patch
import nose.tools

def test_save_fails_if_operation_failure(self):
        handler = Whatever()
        with patch.object(handler, "_save") as _save_mock:
            with nose.tools.assert_raises(OperationFailure):
                _save_mock.side_effect = OperationFailure("message")
                result = handler._save({'key': 'value'})
                self.assertEqual(result.code, 'ERROR')
                self.assertEqual(result.message, MESSAGE)

Test is OK of course


@nedbat
Copy link
Owner Author

nedbat commented Apr 9, 2014

This test looks wrong to me. You are inside an assert_raises clause, which means you believe your hander._save() function call will raise that exception. If you test passes, it must mean that your function call actually raised that OperationFailure exception. Therefore your last two assertEqual statements are never executed.

You have to decide if you want the function to raise the exception, in which case keep the assert_raises and remove the assertEquals, or you want the function to return a value, in which case remove the assert_raises.

It could be that you are importing OperationFailure differently in the test code and the product code, and that is why the except clause in the product code isn't catching the exception.

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

Original comment by lc3t35 (Bitbucket: lc3t35, GitHub: lc3t35)


Thank Ned for your review, you are absolutly right, last two assertEqual are not executed.
Let's modify the code :

#!python

try:
    _id = self.collection.save(something)
    if _id is not None:
        return Result(RESULT_OK)
# Missing starts here
except OperationFailure as e:
    print "OperationFailure : code=" + e.code + ", details=" + e.details
    result = Result(RESULT_ERROR, MESSAGE)
# Missing ends here

and the test :

#!python

def test_save_fails_if_operation_failure(self):
        handler = Whatever()
        with patch.object(handler, "_save") as _save_mock:
            with nose.tools.assert_raises(OperationFailure) as cm:
                _save_mock.side_effect = OperationFailure(0, code=1, details="message")
                handler._save({'key': 'value'})
        ex = cm.exception
        self.assertEqual(ex.code, 1, 'OperationFailure code')
        self.assertEqual(ex.details, "message", 'OperationFailure sent a message')
        handler._close()

Test is OK but coverage indicates the 3 last lines are not covered

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

OK, glad we got to the bottom of it.

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

Original comment by lc3t35 (Bitbucket: lc3t35, GitHub: lc3t35)


Hi Ned, there is still the problem of the lines not covered ?

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

The last three lines in your product code are marked as not run because they are not actually run. The only reason your assert_raises test succeeds is because your product code actually raises that exception. When I say it raises it, I don't mean that it raises it and catches it and handles it. I mean that the exception actually is raised from your function into the caller. Your except clause is not executing in your product code.

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

If the three lines of your function were executing, then your function would complete normally, which will make your assert_raises call fail your test. But your test isn't failing.

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

Original comment by lc3t35 (Bitbucket: lc3t35, GitHub: lc3t35)


Do you know a way to raise and catch the OperationFailure Exception so the product code is covered ?

@nedbat
Copy link
Owner Author

nedbat commented Apr 11, 2014

Usually when this happens, it's because you've imported the name OperationFailure in two different ways.

In your code, there are three references to OperationFailure: 1) the actual code that raises the exception (probably in your database library), 2) in the except clause in your product code, and 3) in the assert_raises function call in your test. It seems that #1 and #3 are referring to the same class, and #2 is referring to a different class.

Closely examine how you are importing the exception to see if they are being imported the same.

@nedbat
Copy link
Owner Author

nedbat commented Apr 12, 2014

Original comment by lc3t35 (Bitbucket: lc3t35, GitHub: lc3t35)


Both are declared as

from pymongo.errors import PyMongoError, OperationFailure

I replaced both with import pymongo.errors and use pymongo.errors.OperationFailure instead, same behaviour.

The exception is raised inside the test but not handled in the product code.
The final proof : If i remove the exception from the product code, the test still pass.
Then it's not a "coverage" problem -> thank you very much for your help on this "out of scope" subject.
i have submitted the discussion here : http://stackoverflow.com/questions/23033804/how-can-you-test-your-operationfailure-exception-code

@nedbat nedbat closed this as completed Apr 12, 2014
@nedbat nedbat added major bug Something isn't working labels Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant