-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Add Go tests #440
test: Add Go tests #440
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, some questions added, but nothing super important.
.gitignore
Outdated
@@ -56,6 +54,19 @@ clients/php/test/Api/* | |||
!clients/php/test/Api/LocalesApiTest.php | |||
!clients/php/test/Api/UploadsApiTest.php | |||
|
|||
clients/go/*.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've discussed this here #427 (comment) and decided not to go this individual exclusion route. so, feel free to leave this file as is, and only git add -f
files that need to be added.
clients/go/test/api_locales_test.go
Outdated
apiClient := phrase.NewAPIClient(configuration) | ||
|
||
|
||
t.Run("Test LocalesApiService AccountLocales", func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd delete these unused tests.
clients/go/test/api_locales_test.go
Outdated
t.Run("Test LocalesApiService LocalesList", func(t *testing.T) { | ||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
// Send the mock response | ||
response := `[{"foo": "bar"}]` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also return a proper format and actually assert that returned locales are deserialized properly?
clients/go/test/api_uploads_test.go
Outdated
apiClient := phrase.NewAPIClient(configuration) | ||
|
||
t.Run("Test UploadsApiService UploadCreate", func(t *testing.T) { | ||
localVarOptionals := phrase.UploadCreateOpts{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how easy would it be to pass an actual file to this call and then assert that the body contains it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked into it, tried to access request via http.response object
httpRes.Request.Body
But it returned {}
Also tried to explore possibility to read the request Body inside the mock server but I will have to look further how to handle the returned Body object of type io.ReadCloser
which by default is unreadable. If its important, I can further investigate if its possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ✅
Approved by Sonke and Mladen is on vacation.
Add basic tests for Go Lang.
Related Issue: https://phrase.atlassian.net/browse/TSI-2123