Http Assertions does not allow the creation of a body

[ihulsbus]

Hi,

It seems like the HTTP Assert package does not allow me to specify a body to pass in a POST request for example. Looking at the HTTPBody() function, the body argument is set to nil:
req, err := http.NewRequest(method, url+"?"+values.Encode(), nil)

Is this by design, or am I looking at a missing feature?

Cheers,

[mvdkleijn]

We feel that it's reasonable to want to be able to set the body in certain cases. If you'd like to help out, feel free to open a PR.

[gohargasparyan]

Hi @mvdkleijn , I just made changes for the issue, tried to push to a new branch and got 403, do I need some permissions to make PR?

[mvdkleijn]

Hi @mvdkleijn , I just made changes for the issue, tried to push to a new branch and got 403, do I need some permissions to make PR?

This sounds like you're new or perhaps I'm misunderstanding? 😄

The way you create a PR is by forking the repository to your own copy, create a branch with your changes pushed to it and then use the Github UI to create the PR. Basically that will be a request for us to merge changes from your fork into our original copy.

It sounds like you were trying to push to a branch in the main Testify repository which is indeed off limits to all but the maintainers.

Hope that helps!

[gohargasparyan]

Yeah first time trying to contribute to open source, thanks, it helps :)

[mvdkleijn]

Yeah first time trying to contribute to open source, thanks, it helps :)

No worries, feel free to ask more if desired. The Github docs also provide a lot of information on how to do things.

Once you have the PR in place, I'll take a look and see if anything needs adjustment and once satisfied all looks good, we can merge it. 👍

[gohargasparyan]

No worries, it was straight forward. #938

[fazypng]

@mvdkleijn hi i need help. very much help. i just joined the Github community yesterday and i don't understand a single thing I've been reading here 😩

[mvdkleijn]

@mvdkleijn hi i need help. very much help. i just joined the Github community yesterday and i don't understand a single thing I've been reading here 😩

I'm not sure what exactly your question is that I could help with...?

[lwlach]

should this issue be closed?

[mvdkleijn]

Not really, since the merged PR was reverted by @boyan-soubachov and aa such the issue is still valid.

[ossan-dev]

Hello @boyan-soubachov,
if you could share the details, I'm gonna file a PR to integrate this functionality.
Let me know, thanks.

[dolmen]

@ossan-dev See #938.

[ossan-dev]

Hey @dolmen
do you mean this issue could be closed as soon as #938 will be merged?
I apologize for the silly question but I'm a beginner of open-source contributions.

[hendrywiranto]

Hey @dolmen do you mean this issue could be closed as soon as #938 will be merged? I apologize for the silly question but I'm a beginner of open-source contributions.

see the details in #938 (comment)

[myusko]

Is an issue still relevant? cc @arjunmahishi

[arjunmahishi]

@myusko Looks like the linked PR's comments are still not addressed.

Is an issue still relevant?

Yes. The request raised in this issue is still not satisfied.

---

Based on my understanding of reading the comments on this issue and the PR, it looks like the PR was raised with v2 in mind. But that might not be happening. So, the next logical step for this would be to follow either suggestion 'b' or 'c' in this comment. 'c' being the best case scenario.

[st3penta]

Any ideas on how this could be done without introducing breaking changes?

I can't seem to find a solution that doesn't involve an extension of the surface of the library (the b scenario, in this comment)

[brackendawson]

#1491 is related. What do the HTTP handler assertions offer over calling the handler with a httptest.ResponseRecorder?

[st3penta]

The only http assertions available right now are on the response status codes (HTTPSuccess, HTTPRedirect, HTTPStatusCode, etc) and body (HTTPBodyContains, HTTPBodyNotContains)
(I hope this is what you asked, i'm not sure i understood your question!)

[brackendawson]

I mean, why are the HTTP assertions better than this:

r := httptest.NewRequest(http.MethodGet, "http://example.com/biscuit", nil)
w := httptest.NewRecorder()
myHandler(w, r)
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, "custard cream", w.Body.String())

This only calls the handler once, so is a better test as it doesn't allow a stateful handler to sneak incorrect behaviour past the test. Like returning status code 500 with a correct body for every request after the first, for example.

[st3penta]

I agree that your example is much more effective.
However those methods are part of the promise too, so what we're supposed to do with them?

[brackendawson]

I agree that your example is much more effective. However those methods are part of the promise too, so what we're supposed to do with them?

Maintain the existing ones. 🙂

[KianYang-Lee]

I mean, why are the HTTP assertions better than this:

r := httptest.NewRequest(http.MethodGet, "http://example.com/biscuit", nil)
w := httptest.NewRecorder()
myHandler(w, r)
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, "custard cream", w.Body.String())

This only calls the handler once, so is a better test as it doesn't allow a stateful handler to sneak incorrect behaviour past the test. Like returning status code 500 with a correct body for every request after the first, for example.

Suggest to include this example in documentation and close this issue

[brackendawson]

Since we're not accepting more assertions which call a handler I'm moving this to close.

I've been thinking about where the above doc should live and I can't think of a good location in the package. Please do open a PR if you can think of one. Perhaps it would be better suited to a guide in the discussions section. Or possibly @Antonboom do you think testifylint should warn about use of the HTTP assertions and suggest it?