Skip to content
This repository was archived by the owner on Nov 14, 2024. It is now read-only.

chore: refactor test helper function for HTTP API tests#8

Merged
johnstcn merged 1 commit intomainfrom
cianjohnston/test_helper
Aug 25, 2021
Merged

chore: refactor test helper function for HTTP API tests#8
johnstcn merged 1 commit intomainfrom
cianjohnston/test_helper

Conversation

@johnstcn
Copy link
Copy Markdown
Member

There was a bit of repeated code in the unit tests that can be refactored into a helper function.

@johnstcn johnstcn requested a review from jawnsy August 25, 2021 13:54
@johnstcn johnstcn self-assigned this Aug 25, 2021
func (k *KubernetesChecker) CheckResources(ctx context.Context) []*api.CheckResult {
results := make([]*api.CheckResult, 0)
return results
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually shouldn't be in here yet 😅

err := json.NewEncoder(w).Encode(test.Response)
assert.Success(t, "failed to encode response", err)
}))
server := newTestHTTPServer(t, http.StatusOK, test.Response)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering whether we should override the client's RoundTripper instead of creating a test server, but 🤷‍♂️

seems good to me!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you can do some fun stuff with that. I especially like the look of NewFileTransport (https://pkg.go.dev/net/http#NewFileTransport).

If (when?) we end up needing to do lots of complex request/response interactions in unit tests, it might be worthwhile looking into this.

@johnstcn johnstcn merged commit e8be680 into main Aug 25, 2021
@johnstcn johnstcn deleted the cianjohnston/test_helper branch August 25, 2021 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants