Skip to content

Conversation

@gnprice
Copy link
Member

@gnprice gnprice commented May 24, 2023

The main ingredient here is that we have FakeHttpClient log the last http.Request object it was passed, and we add some package:checks extensions so that tests can conveniently inspect that request.

We also fix a small bug in ApiConnection.postFileFromStream, which writing these tests uncovered: it was doubling the slash at the start of the URL path.

This makes another step toward #37.

gnprice added 11 commits May 23, 2023 15:18
This includes a "TODO(checks)" comment, indicating a question
that should go upstream for the `package:checks` beta.
This will make things a bit cleaner later, when we accept more parameters.
Also unpack a TODO comment into a softer invitation to expand the
API, and a list of suggestions.

(This wasn't really an actionable TODO in the first place; its real
function was always more to just disclaim any belief that the API was
sufficient, as an invitation to go ahead and expand it when needed.)
Seems most logical, given that the account contains the auth.
And this will enable us to test that the auth information gets
appropriately included in the HTTP request.

(The API key should of course be a fake one; the tests shouldn't
have any other kind of API key in the first place.  And the
fake connection won't generate any real network requests in
any case, because it uses a FakeHttpClient.)
Caught this when I wrote tests for this method and its peers.
Those tests are coming in the next commit.
@chrisbobbe
Copy link
Collaborator

LGTM, thanks! Merging.

@chrisbobbe chrisbobbe merged commit 70f49a4 into zulip:main May 24, 2023
@chrisbobbe
Copy link
Collaborator

We also fix a small bug in ApiConnection.postFileFromStream, which writing these tests uncovered: it was doubling the slash at the start of the URL path.

(Nice!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants