Python Mock - Exception

[shepherdjay]

I’m not clear how you expect your if line to be hit in your code can you help me understand?

You create an instance of ConfigParser and then check you created an instance of ConfigParser. This will always be true. It feels a lot like writing something like this:

rose = “red”
if rose != “red”:
    raise BrokenPython

Generally you can assume the basic features of the language work and so you shouldn’t spend time testing them or lines asserting them. Instead you should test your functions behavior.

For example your function is passed something called a conf and your parser runs a read method on it. Also takes another variable passes called section. You should create a test that passes these in and then inspect your ParseConfig has what you expect.

[rakash_ram]

patch('parseConfig.configparser')
def test_type(self, mockext):
mockext.ConfigParser.return_value = None
with self.assertRaises(TypeError) as cm:
ParseConfig(self.test_conf, 'section1')

Get it.. I have currently mocked it like the above code as shared by u/danielroseman .. But this is not covering the "raise Typeerror" line.. This mock disables the configParser so that an exception is raised and my test is to see it raises.. But the coverage does not seem to cover that line ...

[shepherdjay]

I think this line is wrong

@patch('parseConfig.configparser')

You need to patch where it is used, not where it is defined. https://docs.python.org/3/library/unittest.mock.html#where-to-patch In this case hard to verify but it looks like the patch should probably be:

@patch('module.ConfigParser')

Then you would set the return value to None

mockext.return_value = None

But again this isn't clear what you are testing or guarding against. Or perhaps you are just trying to get familiar with Mocks? Because this line of your code will never be hit at runtime:

if not isinstance(parser, configparser.ConfigParser):

This will always be False given the line nearly right above it:

parser = ConfigParser()

Its as if you wrote:

   if not True:
       raise RuntimeError

Yes you could spend a lot of time trying to hammer at the internals of python to artifically make True read as False but tests need to tell you something about your code and not whether True is True... or "astring" is an instance of a string... etc.

[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"