-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: fetch user credentials in sdk #78
Conversation
default: | ||
return nil, fmt.Errorf("error: list credentials unexpected status code: %d", resp.StatusCode) | ||
} | ||
} |
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.
No test of this function?
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.
None of the cloudian invoking endpoints have tests. We would have to introduce some interfaces and start mocking or stubbing I guess.
I've only tested curl against the cloudian API docs
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 should probably add some kind of Go REST module that we can easily stub out. Untested code is no good.
Co-authored-by: Erik Godding Boye <[email protected]>
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, but I don't like that a lot of the code is untested.
) | ||
|
||
func TestSecretUnmarshal(t *testing.T) { | ||
jsonString := `[{"accessKey":"124","secretKey":"x+2","createDate":1735894172440,"active":true}]` |
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.
Don't think we need to test the builtin go marshalling. Suggesting to simply remove the test.
Obfuscating also seems a bit overkill, as the thing running this has access, and we should not print the thing in production.
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.
The sdk func LGTM! Suggesting to merge just that, and remove secret.go
and secret_test.go
and focus on the important parts now. Add tests when they provide real value.
Co-authored-by: Amund Tenstad <[email protected]>
Co-authored-by: Amund Tenstad <[email protected]>
Merging as-is with both |
No description provided.