4

I am trying to write unittests for a function I wrote that includes a try/except block.

I have raised a custom exception if my string contains less than 3 characters. I wanted to test that the error gets raised when inputting a string less than 3 characters long.

When running the function, if I input a string less than 3 characters, e.g. "ha" - I get the correct error message: "There are not enough letters in your sentence" which leads me to believe that I have raised the custom exception correctly, however, googling has told me that this means I have not raised my custom exception in my function. I just cannot see or understand where I have gone wrong.

Function file:

from collections import Counter


# set custom exception to raise when a word with less than 3 letters is given
class NotEnoughLetters(Exception):
    pass


# create a function that will return the 3 most common letters
def three_most_common(string: str):
    string = string.replace(" ", "")
    try:
        if not all(char.isalpha() for char in string):
            raise ValueError("Your input must contain a string")  # using all because in this instance I haven't accounted for strings and ints mixed

        if len(string) < 3:
            raise NotEnoughLetters("There are not enough letters in your sentence")

        most_common = Counter(string).most_common(3)
        letters = [key for key, value in most_common]

    except ValueError as err:
        return err
    except NotEnoughLetters as e:
        return e
    else:
        return f"Here are your most common letters: 1) {letters[0]}  2) {letters[1]}  3) {letters[2]}"

    finally:
        print("The program is running, please wait for your output")

Test file:

import unittest
from unittest import TestCase
from common_letters import three_most_common, NotEnoughLetters


class TestCommonLetters(TestCase):

    # valid input
    def test_good_string(self):
        expected_input = "cheesy puff"
        expected_output = "Here are your most common letters: 1) e  2) f  3) c"
        result = three_most_common(expected_input)
        self.assertEqual(expected_output, result)  # add assertion here

    # invalid input
    def test_bad_string(self):
        expected_input = "cheesy puff"
        false_output = "Here are your most common letters: 1) f  2) f  3) e"
        result = three_most_common(expected_input)
        self.assertNotEqual(false_output, result)

    # edge case 1, having 3 letters
    def test_having_three_letters(self):
        expected_input = "hay"
        expected_output = "Here are your most common letters: 1) h  2) a  3) y"
        result = three_most_common(expected_input)
        self.assertEqual(expected_output, result)

    # edge case 2, having 2 letters TODO this didn't work so get clarification tomorrow as to why not
    def test_having_two_letters(self):
        with self.assertRaises(NotEnoughLetters):
            three_most_common(string="ha")


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

This is giving me the following output:

Traceback (most recent call last):
  File "C:\Homework-LivvyW\Homework-LivvyW\Homework_week7-LivvyW\test_common_letters.py", line 31, in test_having_two_letters
    with self.assertRaises(NotEnoughLetters):
AssertionError: NotEnoughLetters not raised

I have tried to look at similar stackoverflow question/answers but sadly still not comprehending why/where I have gone wrong. Thank you!

Matiiss
  • 5,970
  • 2
  • 12
  • 29
Livvy Wilson
  • 45
  • 5
  • 1
    `three_most_common` doesn't raise that exception, that's exactly what the test failure is telling you. – mkrieger1 Jul 05 '23 at 23:10
  • Obviously you think that it raises the exception because you have written the line `raise NotEnoughLetters`, but I don't quite understand how you have consciously also written `except NotEnoughLetters as e: return e` and still think that the function raises the exception. – mkrieger1 Jul 05 '23 at 23:16
  • 2
    Does this answer your question? [Is there any difference between return and raise an Exception?](https://stackoverflow.com/questions/40315882/is-there-any-difference-between-return-and-raise-an-exception) – mkrieger1 Jul 05 '23 at 23:18
  • I’m just trying to learn that’s all, – Livvy Wilson Jul 05 '23 at 23:18
  • @mkrieger1, I think a bit. I watched a video that mentioned raising the error in the try segment so it jumps to the except block if the condition was met and it was formulated a similar way, so I must have misunderstood it. – Livvy Wilson Jul 05 '23 at 23:22
  • 1
    I think you misunderstood what you were taught. You're not supposed to catch exceptions *in the same place* where you raise them. You raise an exception in a function to communicate to the code that is calling your function that it could not perform its job. If you catch the exception in the function, you subvert this. – mkrieger1 Jul 05 '23 at 23:30

2 Answers2

2

You are not raising exception because of this block:

except NotEnoughLetters as e:
    return e

Instead you are catching it and than just returning it. Remove this block and it will be raised. The same is correct for ValueError. With these changes your code becomes:

from collections import Counter

# set custom exception to raise when a word with less than 3 letters is given
class NotEnoughLetters(Exception):
    pass


# create a function that will return the 3 most common letters
def three_most_common(string: str):
    string = string.replace(" ", "")
    try:
        if not all(char.isalpha() for char in string):
            raise ValueError("Your input must contain a string")  # using all because in this instance I haven't accounted for strings and ints mixed

        if len(string) < 3:
            raise NotEnoughLetters("There are not enough letters in your sentence")
            
        most_common = Counter(string).most_common(3)
        letters = [key for key, value in most_common]

        return f"Here are your most common letters: 1) {letters[0]}  2) {letters[1]}  3) {letters[2]}"

    finally:
        print("The program is running, please wait for your output")

With these modifications all your 4 tests pass.

frankfalse
  • 1,553
  • 1
  • 4
  • 17
Andrei Evtikheev
  • 331
  • 2
  • 12
  • Ok so would it be best practice to remove the block or would it be better to test the exception through a different manner than assertRaises? So far I have been taught to catch exceptions in try/except blocks – Livvy Wilson Jul 05 '23 at 23:25
  • Raising an exception and then catching and returning it is definitely not a good practice. IMHO you should just raise it (and test it with assertRaises). Or you can return some kind of an error code in if block and test that function returned that code (not raised an exception) but I think raising is better. – Andrei Evtikheev Jul 05 '23 at 23:31
  • ok than you, @andrei so whilst revising I came across this video: https://youtu.be/NIWwJbo-9_8 in the last two minutes, from 8:30 onwards he raises a custom exception and he says so you can catch it later on, is that wrong or is it different because he uses a print statement? Apologies this is very new to me and quite confusing right now. – Livvy Wilson Jul 06 '23 at 06:01
  • I have done some digging and I might be getting somewhere. So to sort it all out could I simply change the return e to raise e? So that it’s raises the custom exception like I have stated earlier. Or because I have raised it in the if statement should I not raise it at all in the except segment and just catch other potential exceptions within it? – Livvy Wilson Jul 06 '23 at 09:17
1

Not raise the Custom Exceptions

An other possibility is to remove your custom Exceptions an return directly the error message. So your code becomes:

from collections import Counter

# create a function that will return the 3 most common letters
def three_most_common(string: str):
    string = string.replace(" ", "")
    try:
        if not all(char.isalpha() for char in string):
            #raise ValueError("Your input must contain a string")  # using all because in this instance I haven't accounted for strings and ints mixed
            return "Your input must contain a string"

        if len(string) < 3:
            #raise NotEnoughLetters("There are not enough letters in your sentence")
            return "There are not enough letters in your sentence"

        most_common = Counter(string).most_common(3)
        letters = [key for key, value in most_common]

    #except ValueError as err:
    #    return err
    #except NotEnoughLetters as e:
    #    return e
    #else:
        return f"Here are your most common letters: 1) {letters[0]}  2) {letters[1]}  3) {letters[2]}"

    finally:
        print("The program is running, please wait for your output")

How to change the test code

Accordingly the test code becomes the following:

import unittest
from unittest import TestCase
#from common_letters import three_most_common, NotEnoughLetters
from common_letters import three_most_common

class TestCommonLetters(TestCase):

    # valid input
    def test_good_string(self):
        expected_input = "cheesy puff"
        expected_output = "Here are your most common letters: 1) e  2) f  3) c"
        result = three_most_common(expected_input)
        self.assertEqual(expected_output, result)  # add assertion here

    # invalid input
    def test_bad_string(self):
        expected_input = "cheesy puff"
        false_output = "Here are your most common letters: 1) f  2) f  3) e"
        result = three_most_common(expected_input)
        self.assertNotEqual(false_output, result)

    # edge case 1, having 3 letters
    def test_having_three_letters(self):
        expected_input = "hay"
        expected_output = "Here are your most common letters: 1) h  2) a  3) y"
        result = three_most_common(expected_input)
        self.assertEqual(expected_output, result)

    # edge case 2, having 2 letters TODO this didn't work so get clarification tomorrow as to why not
    """def test_having_two_letters(self):
        with self.assertRaises(NotEnoughLetters):
            three_most_common(string="ha")"""

    # edge case 2, having 2 letters TODO this didn't work so get clarification tomorrow as to why not
    def test_having_two_letters(self):
        result = three_most_common(string="ha")
        self.assertEqual("There are not enough letters in your sentence", result)

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

In the test code, the test case test_having_two_letters() verifies the error message returned by the function three_most_common() (as in the other test cases) and not if it is raised an Exception.

frankfalse
  • 1,553
  • 1
  • 4
  • 17