ResponseRecorder HTTP status code defaults to 200

[Simon Walter]

Hi all,

What is the reason that ResponseRecorder HTTP status code defaults to 200?

https://cs.opensource.google/go/go/+/refs/tags/go1.21.3:src/net/http/httptest/recorder.go;drc=ff14e844d26090e09aa335d836f737c09a7a0402;l=55

https://cs.opensource.google/go/go/+/refs/tags/go1.21.3:src/net/http/httptest/recorder.go;drc=ff14e844d26090e09aa335d836f737c09a7a0402;l=196

Can someone explain to me why this is a good idea? Would it not make more sense to set it to a value that is not a valid HTTP response code such as 0?

Simon

[Axel Wagner]

A default `net/http` server also uses 200 as the default response code. The behavior of an `http.Handler` not setting a response code should be the same, if it uses `httptest`, as if it uses `net/http`.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/321a84fa-3aeb-4f3a-ba4f-a05e797652d6n%40googlegroups.com.

[Simon Walter]

How does that explain why it is a good idea?

Perhaps my concern is not clear enough.

For example:

n := 5

funcThatShouldSetNtoFive(&n)

if n != 5 {

  panic("n is not 5)

}

This is not a good test.

I can check if the value has changed by:

n := 0

funcThatShouldSetNtoFive(&n)

if n != 5 {

  panic("n is not 5)

}

That's a bit more sensible.

Wouldn't it be less risky for a test to fail by default? Removal of the call to funcThatShouldSetNtoFive, IMO, should result in failure.

[Axel Wagner]

If you want to argue that `net/http` should not set the response code to 200 by default, I would be inclined to agree. I don't particularly like that the API even *allows* you not to explicitly set a response code. But it does. And while it does, the best test is one that matches the behavior of the production environment as closely as possible, full stop.

Another way to look at it: Why do you believe you have to test that your handler sets the response code to 200? Why should the test fail, if it doesn't do it - given that *the production code* will still end up with the right response code? If the response sent to the user is the right one - why would it matter whether it was your Handler that set it, or the framework?

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/df053fb7-70bf-483d-afd6-4caee9937aa6n%40googlegroups.com.

[Simon Walter]

Axel, thanks for providing some context.

I suppose it is better for me to think of the httptest package as specific to the http package - although this is not explicitly stated: "Package httptest provides utilities for HTTP testing"

This seems misleading as is the case with this '200' default.

I stopped using httptest.NewRecorder() because of the possibility to introduce changes to tests that would make tests pass. Not everyone knows the internals of all the code. Because of this, I think it is risky to have values set by default to the value that causes the test to pass.

Some questions that should not keep us awake at night: The test passed, but will it fail? Will it fail where/how I think it will?

Simon

[Axel Wagner]

It seems to me that the fact that the functions accept and return types from `net/http` (like `http.ResponseWriter` and `http.Handler`) and given that it's nested below `net/http` should make it fairly obvious that it's meant to be used with `net/http`. I also genuinely don't understand what the intersection is of "being tempted to use `httptest`" and "does not intend to be used with `net/http`". I also genuinely don't understand how the behavior of `httptest` could ever cause any harm (quite the opposite).

But, YMMV, of course and you are free to roll your own testing helpers.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/c69a5ab5-a6c7-4609-900d-38e31ddb74bbn%40googlegroups.com.

[Axel Wagner]

To be clear: The behavior of an `http.ResponseWriter` to implicitly use a 200 status code if no explicit WriteHeader call is made is documented:

https://pkg.go.dev/net/http#ResponseWriter.WriteHeader

I really think it is a bad (or at least strange) idea to claim to implement `http.ResponseWriter` while *not* replicating this behavior.

[Simon Walter]

It is good practice to use test lang/tools/libs/etc different from what you are using in your actual code.

If the author has meant for the httptest package as general purpose HTTP testing library, then, I will argue that such defaults are incorrect. It is incorrect even if the intended purpose is to tests code that is derived from http package.

If the author has meant for the httptest package to be used for tests internal to the http package, then it should be labeled such.

I'll mention this to brad...@golang.org.

[Simon Walter]

And so: https://github.com/golang/go/issues/63589

[Axel Wagner]

It is not "meant for tests internal to the http package". It's meant for tests of users of the `net/http` package. That is, implementations of the `http.Handler` interface.

In the context of the standard library, that is what "general purpose HTTP testing" means.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/55f2db46-66f4-42f4-a9ac-56b827073703n%40googlegroups.com.