3

I have this method called str_to_hex in my common.py

def str_to_hex(self, text):
    self.log.info('str_to_hex :: text=%s' % text)
    hex_string = ''
    for character in text:
        hex_string += ('%x' % ord(character)).ljust(2, '0') 
    self.log.info('str_to_hex; hex = %s' % hex_string)
    return hex_string

The unittesting method that I am writing is 

def test_str_to_hex(self):
    # test 1
    self.assertEqual(self.common.str_to_hex('test'), '74657374');
    # test 2
    self.assertEqual(self.common.str_to_hex(None) , '')
    # test 3
    self.assertEqual(self.common.str_to_hex(34234), '')
    # test 4
    self.assertEqual(self.common.str_to_hex({'k': 'v'}), '')
    # test 5  
    self.assertEqual(self.common.str_to_hex([None, 5]), '')

So the first failures that I got say

# failure 1 (for test 2)
TypeError: 'NoneType' object is not iterable
# failure 2 (for test 3)
TypeError: 'int' object is not iterable
# failure 3 (for test 4)
AssertionError: '6b' != ''
# failure 4 (for test 5)
TypeError: ord() expected string of length 1, but NoneType found

Ideally only text (i.e. str or unicode) should be passed to str_to_hex

For handling empty args as input I modified my code with

def str_to_hex(self, text):   
    # .. some code ..
    for character in text or '':
    # .. some code

So it passes the second test but still fails for the third one.

If I use hasattr(text, '__iter__'), it will still fail for test #4 and #5.

I think the best way is to use an Exception. But I am open to suggestions.

Please help me out. Thanks in advance.

Community
  • 1
  • 1
Hussain
  • 5,057
  • 6
  • 45
  • 71
  • Adding to this..do you test for additional datatypes like say a dict or a list as inputs to the method? If yes, how to avoid taking the focus away from the main logic? – hyades Mar 27 '15 at 06:57
  • That's a great point. It will work for a `list` provided it only contains `char`. e.g. ['a', 'b', 'c']. But what about `dict` and other lists? – Hussain Mar 27 '15 at 06:59
  • modified the question – Hussain Mar 27 '15 at 07:10
  • Why do you want an empty string, rather than an exception? That seems like it'll just hide errors and confuse the people who work on this code after you. – user2357112 Mar 27 '15 at 07:13
  • @user2357112 yea it should ideally be throwing out different kinds of `Exception` in the different cases. – hyades Mar 27 '15 at 07:17
  • So what different exception should I handle besides `TypeError` and `AssertionError` in case a method is only able to process **text**? – Hussain Mar 27 '15 at 07:18

1 Answers1

1

First off, you need to decide whether you want to (a) silently return empty strings for invalid input like lists, dicts, etc. OR (b) you actually are fine with raising appropriate exceptions, just want your tests to deal with those.

For (a), you can make your function itself more defensive about what it's getting passed:

def str_to_hex(self, text):
    if not isinstance(text, basestring):
        return ''
    # rest of code

For option (b), you can change your test expectations to match what's happening:

with self.assertRaises(TypeError):
    self.common.str_to_hex(None)
# etc.
tzaman
  • 46,925
  • 11
  • 90
  • 115
  • I have seen that some texts have datatype = `unicode` (e.g. `u'á'`). Will the test fail for such text? – Hussain Mar 27 '15 at 07:44
  • I just tried `self.assertEqual(self.common.str_to_hex(u'á'), '')` against option (a) and it fails with `AssertionError` – Hussain Mar 27 '15 at 07:47
  • `basestring` will allow both unicode and normal strings through. IF you want to disallow unicode as well, just change it to `isinstance(text, str)`. – tzaman Mar 27 '15 at 07:53
  • yes. that's correct. My test should have been `self.assertEqual(self.common.str_to_hex(u'á'), 'e1')`. Sorry :( – Hussain Mar 27 '15 at 07:56
  • I don't think Option-A will work. Option-B is not really an option. Its doesnt explain what we should be doing in the main code. Here `basestring` does the job, but at many places it wont. Will you end up putting in `or` statements like `isinstance(x1) or isinstance(x2)` ?? Also if you do it this way, I think we are concentrating more on the test code rather than the main logic since more than half the lines of code is actually just catching exceptions and throwing them. And this is only the code which test for type (the useless part I would call) since main test logic isn't in this. – hyades Mar 27 '15 at 11:15
  • I agree with you. Maybe I should put the validations in a decorator. That way it will be separate from my actual logic. I too mixing the actual logic and the extra coating. – Hussain Mar 27 '15 at 11:19