Skip to content
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

Remove httpclient.Client interface #6726

Closed
absoludity opened this issue Aug 29, 2023 · 0 comments · Fixed by #6756
Closed

Remove httpclient.Client interface #6726

absoludity opened this issue Aug 29, 2023 · 0 comments · Fixed by #6756
Assignees
Labels
component/ci Issue related to kubeapps ci system kind/enhancement An issue that reports an enhancement for an implemented feature

Comments

@absoludity
Copy link
Contributor

Summary

Remove the httpclient.Client interface that Kubeapps uses to implement fake clients in tests.

Background and rationale
Early on in Kubeapps' history, the httpclient.Client interface was added it seems solely so that test implementations of that interface could be used in tests (with dummy responses).

But this leaks into the actual code, with us passing our own httpclient.Client implementations around which are not standard http.Client structs, and so can't be used as such.

I'm currently working on #6706 and as part of that am needing to integrate with the ORAS-go library (which we've wanted to do anyway), but to do so, need to use and pass a standard http.Client.

Description

Go has an excellent httptest library (not sure if wasn't known about at the time the original Kubeapps tests were written) which removes the need for a fake client, as it instead creates a fake server and you can use your real client, so there is no need for our httpclient.Client interface anymore either.

We should refactor to use that instead

Acceptance criteria

Tests pass without the interface and we remove all use of it.

Additional context

I'm just creating this issue because I started working on this today, but didn't get it finished, so just dumping the rationale for what I'm doing.

@absoludity absoludity self-assigned this Aug 29, 2023
absoludity added a commit that referenced this issue Sep 1, 2023
### Description of the change

Part 1 removing the httpclient.Client interface. See #6726 for which
this is needed.

### Benefits

Able to write code that expects a plain http.Client.
Better tests.

### Possible drawbacks

### Applicable issues

- ref #6726

### Additional information

---------

Signed-off-by: Michael Nelson <minelson@vmware.com>
@ppbaena ppbaena added component/ci Issue related to kubeapps ci system kind/enhancement An issue that reports an enhancement for an implemented feature labels Sep 4, 2023
absoludity added a commit that referenced this issue Sep 5, 2023
### Description of the change

Follows on from #6746 removing more uses of the httpclient.Client test
interface that's causing pain when adding new features, and updates
tests to use an `httptest.Server` instead of mocking the client.

### Benefits

See #6726 

Also fixes more tests that were feigning to assert that errors were
equal, but were simply asserting that an error was raised.

### Possible drawbacks

### Applicable issues

- fixes #6726

### Additional information

~~One more PR following which removes the remainder and the interface
itself.~~ (Ended up just adding to this PR).

---------

Signed-off-by: Michael Nelson <minelson@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ci Issue related to kubeapps ci system kind/enhancement An issue that reports an enhancement for an implemented feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants