sbansla commented 2 years ago

Fixes

Standardisation of UserAgent String as per below mentioned doc https://docs.google.com/document/d/1dDAvu9N4wkTnZKmWx5iiMfU42b7qYLAegA_QbZoBkU0/edit#

Checklist

[x] I acknowledge that all my contributions will be made under the project's license
[x] I have made a material change to the repo (functionality, testing, spelling, grammar)
[x] I have read the Contribution Guidelines and my PR follows them
[x] I have titled the PR appropriately
[x] I have updated my branch with the main branch
[x] I have added tests that prove my fix is effective or that my feature works
[ ] I have added the necessary documentation about the functionality in the appropriate .md file
[ ] I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

shrutiburman commented 2 years ago

Could you link the issue that this PR solves, in the description ?

sbansla commented 2 years ago

Could you link the issue that this PR solves, in the description ?

Added

sbansla commented 2 years ago

High-level feedback on the new tests: they do execute the new/changed lines and have assertions, but the assertions are overly loose. They should assert on the expected impact of the new behavior rather than the new behavior not having a negative impact.

Let me know if you'd like to sync for more discussion/clarification.

---> Corrected the test cases wherever possible. File: TwilioRestClientTest, I have marked variable twilioRestClientExtension as @Injectable. If I don't mark it as @Injectable, Execution of method won't happen. twilioRestClientExtension.request(request) --> Won't succeed without @Injectable This behaviour was strange to me.