search

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 some tests for request handler #113

Closed andyg7 closed 7 years ago

andyg7 commented 7 years ago

Hey guys,

1) I wrote a few tests here for request_handler.py, the new module I refactored out. As of now I just added tests for three methods from the RequestHandler class to get some feedback. I used MagicMock to mock out the api calls, and basically ran a few tests to make sure the proper code was called. Was looking just to get some code coverage going.

2) I also simplified some of the tests from test_load_data.py as per the discussion from #110 that @Saturn suggested.

3) I also added mock==1.0.1 to the requirements.txt file.

4) As mentioned in a previous PR to run all tests run the following command: "python -m unittest discover tests" from the root of the repo. This will go into tests/ and look to run all tests

Thanks for any feedback :) Andy

andyg7 commented 7 years ago

Hey @carlosvargas, Thanks a lot for the feedback. I replaced the raised checks with a assertFails in all cases, and also added some checks on the error messages that are reported when an exception is. Let me know of any more feedback. Thanks so much, Andy

andyg7 commented 7 years ago

Hey guys, should I squash my commits whenever I make a change? I worry that re-writing history could mess things up (even though it's my forked version) but it also seems annoying to have a lot of commits with tiny changes.

carlosvargas commented 7 years ago

I personally like to do that on PRs, but it's not a big deal.

Everything looks good to me 👍 Let's wait on @Saturn's input.

Saturn commented 7 years ago

Looks good.