Conversation
KaushikiAnand
commented
Mar 13, 2025
KaushikiAnand
commented
- I have added unit test coverage and linting to the ci.yml workflow.
- The unit test coverage change was done to help understand the defects early in lifecycle and come up with a good solution for it. The linting was done to ensure that the codebase is consistent and maintainable and helps in improving the code quality and reducing errors.
.golangci.yml
Outdated
| - errcheck | ||
| disable: | ||
| - unused | ||
| output_format: colored-line-number No newline at end of file |
There was a problem hiding this comment.
Also, why did we need to disable unused ?
request.go
Outdated
| } | ||
| if err := json.NewDecoder(buf).Decode(relationship); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Question: Can you check if the same pattern is happening for other error scenarios ?
I saw at some places, we are setting the value of er and letting the flow either break or continue
There was a problem hiding this comment.
followed the same pattern on suggested areas.
There was a problem hiding this comment.
please add a break for all newly added errors
There was a problem hiding this comment.
added break to newly added errors
|
|
|
|
||
| func TestHttpErrorWhenMethodDoesNotMatch(t *testing.T) { | ||
| r, err := http.NewRequest(http.MethodPatch, "/blogs", nil) | ||
| r, err := http.NewRequest(http.MethodOptions, "/blogs", nil) |
There was a problem hiding this comment.
This test is currently failing in the main branch. It is failing because previously there wasn't any handling for the method http.MethodPatch but around an year back code was added for this case as well.
I have changed the http method to an unhandled path, so that the test case runs as expected
|
|
||
| type OneOfMedia struct { | ||
| Image *Image | ||
| random int |
|
|
||
| func TestUnmarshalToStructWithPointerAttr_BadType_IntSlice(t *testing.T) { | ||
| out := new(WithPointer) | ||
| type FooStruct struct{ A, B int } |
| } | ||
| videoField, ok := result["videos"] | ||
| if !ok || videoField.FieldNum != 2 { | ||
| if !ok || videoField.FieldNum != 1 { |
There was a problem hiding this comment.
An unused field was removed, thus the FieldNum for videoField decreased
| json.NewEncoder(buf).Encode(data.Relationships[args[1]]) //nolint:errcheck | ||
| json.NewDecoder(buf).Decode(relationship) //nolint:errcheck | ||
|
|
There was a problem hiding this comment.
Comment: We are ignoring the lint error to preserve behaviour.
5 participants
