DX-2684 Integration Tests Calls API#94
DX-2684 Integration Tests Calls API#94brianluisgomez merged 24 commits intofeature/openapi-generator-sdkfrom
Conversation
brianluisgomez
changed the base branch from
main
to
feature/openapi-generator-sdk
bpateldx
left a comment
bpateldx
left a comment
There was a problem hiding this comment.
Don't see any major issues. Some room for refactoring and minor updates. Note that 8 Tests failed locally for me, possible misconfiguration!
test/integration/test_calls.py
Outdated
| from bandwidth.model.update_call import UpdateCall | ||
| from bandwidth.exceptions import ApiException, UnauthorizedException, ForbiddenException, NotFoundException | ||
|
|
||
| try: |
There was a problem hiding this comment.
+1 on one-time setup for all Tests.
How can this be refactored to a Test util function?
There was a problem hiding this comment.
To clarify the comment further - Thinking from OOP perspective we've a Test class where all Test methods are encapsulated and then we've other "one-time" setup code above class definition! This can be either moved to @classmethod(suggested) or externalized to a static utility function lib.
| priority=5, | ||
| tag="tag_example", | ||
| ) | ||
|
|
There was a problem hiding this comment.
What do you think about replacing the setup block by a function call (ex. create_call_body)? In case on replaced function call, how much of the relevant info should stay in calling function?
There was a problem hiding this comment.
We could do that, but this particular call body is only used once for this initial test to make sure that we can send all of these parameters. The future call bodies that I use in the tests are smaller and I am making a separate global variable that gets reused for that.
There was a problem hiding this comment.
Agreed on it being not re-used. The suggestion is to reduce ~30 lines to a line(or two) so that it's easy to understand the test structure at high level. Doing it will help fellow developer to easily understand the test setup (create call), action under test and validations. To dig more into create_call_setup one simply click open the implementing method.
test/integration/test_calls.py
Outdated
| self.assertIsInstance((get_call_response[0].direction), CallDirectionEnum) | ||
| self.assertIs(type(get_call_response[0].enqueued_time), datetime.datetime) | ||
| self.assertIs(type(get_call_response[0].last_update), datetime.datetime) | ||
| self.assertIs(type(get_call_response[0].start_time), datetime.datetime) |
There was a problem hiding this comment.
Is it worth checking that start_time is after our first call and last_update to be after start_time?
bpateldx
left a comment
bpateldx
left a comment
There was a problem hiding this comment.
👍 for all refactoring and Test assertions using hamcrest matchers
3 participants
No description provided.