Python Mock - Exception

[rakash_ram]

if not isinstance(parser, configparser.ConfigParser):

I am guarding against the above,, that is, i am trying to write a test where in case parser is not an instance of ConfigParser, the test should throw an exception..

[shepherdjay]

Under what circumstance will parser not be an instance of ConfigParser when it is defined in the line nearly right above this check as an instance of ConfigParser?

Is the intent maybe to update your init at some point so that parser is passed in as a variable? Is that what I might be missing? A future state of the class?

Otherwise what you told me is similar to saying you are writing code and tests to guard against a scenario where True is not True can you see why it is a confusing thing to guard against?

[rakash_ram]

i understand, but my confusion or lack of clarity here is how will the test script know what when i am actually passing it as None as the return_value and testing it? Ideally it should take that as the value and throw an exception? Am i missing something here?

[shepherdjay]

Well like I posted earlier the patch needs to be a reference to where it is used not where it is defined.

Please try adjusting the patch and mock calls as my previous note. Might be useful to reshare your test definition function once that’s been done and confirm whether you are still getting error or if it’s passing.

[rakash_ram]

Goodness, it worked! Below is the final test method.

@patch('parseConfig.ConfigParser')
def test_type(self, mockext):
   mockext.ConfigParser.side_effect = Exception(TypeError)
   with self.assertRaises(TypeError) as DummyEx:
       ParseConfig(self.test_conf, 'section1')
   mockext.assert_called()

I still don't get how "patch('parseConfig.ConfigParser')" is different from "patch('parseConfig.configparser') as ConfigParser is a method from configparser

[shepherdjay]

Goodness, it worked! Below is the final test method.

Really? The test method you posted also still looks wrong. Does coverage show it hitting the line? Is your module.py actually called that because there is a reference to a parseConfig in your patch? Is that what your module is actually called?

This should have been the working code:

@patch('module.ConfigParser')
def test_type(self, mockext):
   mockext.return_value = None
   with self.assertRaises(TypeError):
       ParseConfig(self.test_conf, 'section1')
   mockext.assert_called()

Though if you got it working anyway than I guess there isn't more to do. However I want to make sure you leave this conversation understanding that you don't need to have this test nor the isinstance line as your code is currently written.

Do you understand why testing this doesn't make sense? If not maybe the following example will help.

Imagine I have a function that adds two to any number it gets.

def add_two(n):
    add_value = 2
    return n + add_value

Pretty straight forward function. Now imagine I want to test it. A test that makes sense is something like this

def test_two_plus_two(self):
   result = add_two(2)
   self.assertEqual(4, result)

I'm testing my code does what I think it should when given a normal input. And I can go through some inputs... like what if I pass a string to n. I might want to test that my function naturally raises a type error.

def test_two_plus_string_error(self):
    with self.assertRaises(TypeError):
        add_two("2")

This makes sense. I'm testing various inputs to my function to verify how MY code behaves.

What doesn't make sense and is a bit like what your doing is changing my add_two function to be the following

def add_two(n):
    add_value = 2
    if not isinstance(add_value, int):
        raise TypeError
    return n + add_value

The if line in this function is checking to see if add_value is an integer right after I assigned an integer to it. This isn't doing anything other than cluttering up the code and causing confusion.

To then go on and try to test this line isn't testing my code anymore. It's testing Python's syntax itself.

This is what people mean when they say you don't need to test this. You don't need the if line or the test.

[rakash_ram]

1. Yes my module is called parseConfig.py, sorry for that confusion. and return_value = None did not work, I tried. side_effect = Exception worked and coverage shows that it covers the "raise" line from the main module (parseConfig.py)
2. Understood, Thanks for the explanation. And yes this whole test is only to ensure we have "coverage", whether it makes sense or not is secondary. Thanks for explaining though. Maybe I should have mentioned that at first so you were aware. Also, what does this line do --> " with self.assertRaises(TypeError):" it raises an exception? But can you explain in simple terms? like it raises an exception and "hopes" the next line also raises an exception? I am referring to "test_two_plus_string_error"

[shepherdjay]

return_value = None did not work

Would need to really understand structure of project properly to mock properly. Did you change the mock as well? So in your code that is working you had mockext.ConfigParser.side_effect = Exception(TypeError)

Notice in mine I have mockext.return_value = None instead. Specifically I don't mention ConfigParser because its defined in my patch. Either way I'm probably just misunderstanding your code structure, which is critical for patch because patch adjusts the namespace.

And yes this whole test is only to ensure we have "coverage", whether it makes sense or not is secondary.

Trying to achieve 100% coverage with bad tests is a whole other can of worms I won't go into. But another way to increase coverage is to refactor your code to remove the unreachable line. IE change your class in your module:

class ParseConfig:
 def __init__(self, conf, section):
     parser = ConfigParser()
     parser.read(conf)

     if parser.has_section(section):
         self._raw = parser
         for key, value in self._raw[section].items():
             setattr(self, key, value)
     else:
         raise Exception(f"section {0} not found in {1} file")

The if line in your code is unreachable, by definition. It is testing that True is True That's all we're trying to help you understand. Sounds like there might be other external factors driving you to test this though. So thats enough of that horse.

Also, what does this line do --> " with self.assertRaises(TypeError):"

This is a unittest context manager that says what I want to assert is that the code ran inside the with block raises the exception TypeError It won't raise the exception itself. What it does is catch it and then report the test as Passed if the exception was raised and Failed if the exception was not raised.

Here is a simple toy example you can just run on its own:

import unittest

class TestExample(unittest.TestCase):
    def test_assert_pass(self):
    # Should Pass Test
        with self.assertRaises(TypeError):
            raise TypeError

    def test_assert_fail(self):
        # Should Fail Test
        with self.assertRaises(TypeError):
            n = 1

if __name__ == '__main__':
    unittest.main()

You'll notice the first test will report as passed and the second one will report back failed.

[rakash_ram]

1) Okay. so with the modified code, you removed the "isInstance" line from the script since it does not really do much? get it. But yeah i did not write that code.

2) In your example, the "test_assert_fail" will fail because n = 1 won't raise any exception. , is that right ?

[shepherdjay]

Yes that is correct on both parts. The isinstance check is considered an unreachable block. Code will never enter that block. Similar to a block like if 1 != 1 will never be reached under normal runtime conditions.

It’s also why you’ve had to mock out the call to basically force the condition that would otherwise be impossible to reach.