feat: add support for region and edge values in url #62
Conversation
| @@ -20,7 +21,8 @@ func NewClient(accountSid string, authToken string) *twilio.Client { | |||
| c := &twilio.Client{ | |||
| Credentials: creds, | |||
| HTTPClient: http.DefaultClient, | |||
| BaseURL: "twilio.com", | |||
| Edge: os.Getenv("TWILIO_EDGE"), | |||
There was a problem hiding this comment.
The test shouldn't be reading these values unless it's also clearing them before each test. Otherwise tests may fail (or pass in error) when set in the testing environment.
There was a problem hiding this comment.
If I add os.Setenv("TWILIO_EDGE", ""), os.Setenv("TWILIO_REGION", "") at the end of each the region/edge tests, that should work
There was a problem hiding this comment.
Sort of, but this is partially testing the logic that the test client can read from the env vars (not the actual client). I say avoid it if possible. Unless, for instance, you're testing that the actual client can read from the env vars.
There was a problem hiding this comment.
Gotcha. I was trying to test precedence, but I see that's already handled when setting c.Region or c.Edge, which is equivalent to setting those fields during instantiation of the client. I'll rip out the extraneous tests and submit a new PR
Related JIRA: https://issues.corp.twilio.com/browse/DI-960