architv / soccer-cli

:soccer: Football scores for hackers. :computer: A command line interface for all the football scores.
MIT License
1.09k stars 222 forks source link

Implement simple unit tests and refactor of main.py #110

Closed andyg7 closed 7 years ago

andyg7 commented 7 years ago

I wrote these tests just to get the ball rolling in terms of implementing unit tests. Let me know of any feedback! Thanks :) Andy

andyg7 commented 7 years ago

Hey guys, I refactored the main.py as I mentioned in #111 here. Let me know of any feedback :) Thanks so much, Andy

Saturn commented 7 years ago

I like this.

Although in the test file is there a need to catch the exceptions?

def test_load_team_data(self):
    raised = False
    try:
        load_json(TestLoadData.TEAMS_INFO_FILENAME)["teams"]
    except IOError:
        raised = True
    self.assertFalse(raised)
def test_load_team_data(self):
    load_json(TestLoadData.TEAMS_INFO_FILENAME)["teams"]

I suppose in the case of an IOError the test runner will call the latter an error rather than a failure. But does it matter?

andyg7 commented 7 years ago

Hi @Saturn thanks for the response. The idea of the simple test cases was just to check that the required files do indeed exist (e.g. leagueids.py and leagueproperties.py). If they don't exist then the load_json(file) function will throw an exception. So I figured we could test that the files do exist by checking that an exception is not thrown; hence the try-catch. Does that make sense? I think I'm going to mock out the loading of these two files so that the test suite isn't reliant on them.

Saturn commented 7 years ago

I just didn't think it was totally necessary to wrap the individual tests in try except blocks.

If an IOError occurs just let it happen and the test runner will record it as an error.

And if you want to catch the exceptions within a test, you could do something like this instead.

def test_load_team_data(self):
    try:
        load_json(TestLoadData.TEAMS_INFO_FILENAME)["teams"]
    except IOError:
        self.fail("File doesn't exist!")

Instead of creating flag variables. Just less typing.

dfrt82 commented 7 years ago

At least the split-up of the main file looks good to me. Good work @andyg7 !