GroupMe fetch methods return vertical model instances #73
Conversation
| mock_groupme_transfer_service.fetch_blocked_user_vertical() | ||
|
|
||
|
|
||
| def test_fetch_blocked_user_vertical(mock_groupme_transfer_service, mocker): |
There was a problem hiding this comment.
I'm trying to figure out what this test does test, other than the test harness? It tests mock_groupme_transfer_service which is test harness mock code, it tests 'dump_and_filter_model_objs" which is test utility code, and it tests 'oauth2_session_get' which is mock code again.
There was a problem hiding this comment.
I think what I'm missing here is that 'mock_groupme_transfer_service' is NOT a mock. It's an instance constructed for test purposes with test values, but it's a real instance of GroupMeTransferService and it's the object under test here. Maybe call it "groupme_transfer_service_test_instance" -- that's long but if you want to have the whole name of the core class here. I also think it would be fine to call it "groupme_test_instance" or "groupme_transfer_service". It's pretty common for the instance of a class to be named directly after the class but in snakecase, without having to even call it an instance.
There was a problem hiding this comment.
I agree! These test variable names could be a lot better
#74
Closes #63
Implements parsing for all the verticals supported by GroupMe:
Also added
namefield to ChatBot since bots can have names.Also modifies README with updated vertical-service support.