-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(audio): fix audioTextResponse decode #638
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #638 +/- ##
=======================================
Coverage 98.43% 98.43%
=======================================
Files 24 24
Lines 1342 1345 +3
=======================================
+ Hits 1321 1324 +3
Misses 15 15
Partials 6 6 ☔ View full report in Codecov by Sentry. |
@WqyJh thank you for the PR! Could you also please add a test for that? |
Added test. |
@WqyJh thanks! There are some linter problems now |
client_test.go
Outdated
if err == nil { | ||
t.Error("Unexpected nil error") | ||
} | ||
} else { |
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 can reduce the amount of else
by using early return pattern:
if something {
// code
return
}
// else block
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.
Of course! It would be better to use github.com/stretchr/testify/assert for testing, as it eliminates the need for if/else statements for assertions. However, I didn't use it because it doesn't have any external dependencies. Would you like to use it or keep it dependency-free?
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.
@WqyJh staying dependency free is important for the project, so let's not use the library. We have an internal lib of helpers though: https://github.com/sashabaranov/go-openai/blob/master/internal/test/checks/checks.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.
Thank you!
The audio api passes
audioTextResponse
tosendRequest
,but it cannot be asserted as
*string
type, lead to decoding failure.Fix this problem.