gin-gonic / gin

Gin is a HTTP web framework written in Go (Golang). It features a Martini-like API with much better performance -- up to 40 times faster. If you need smashing performance, get yourself some Gin.
https://gin-gonic.com/
MIT License
78.44k stars 8.01k forks source link

Wrong status code returned when using httptest.NewRecorder #1120

Open emiguens opened 7 years ago

emiguens commented 7 years ago

When testing a gin handler using a httptest.NewRecorder, when using (inside the handler) c.Status(404) to set the status code, and then bailing out of the request, then the status from the recorder is wrongly returned as 200.

On the other hand if we use c.JSON(404, gin.H{}) then the status code inside the recorder is correct.

The expected behavior is of course to always receive in the recorder the status that was set inside the handler.

Example code reproducing the bug bellow, both tests are expected to PASS but only the one using JSON does.

Having a main.go with the following code:

package main

import (
    "net/http"
    "os"

    "github.com/gin-gonic/gin"
)

func main() {
    port := ":" + os.Getenv("PORT")

    r := gin.Default()
    r.GET("/fail1", failStatus)
    r.GET("/fail2", failJson)
    r.Run(port)
}

func failStatus(c *gin.Context) {
    c.Status(http.StatusNotFound)
}

func failJson(c *gin.Context) {
    c.JSON(http.StatusNotFound, gin.H{"error": true})
}

And a main_test.go:

package main

import (
    "net/http"
    "net/http/httptest"
    "testing"

    "github.com/gin-gonic/gin"
)

func TestFailStatus(t *testing.T) {
    gin.SetMode(gin.TestMode)

    rec := httptest.NewRecorder()
    c, _ := gin.CreateTestContext(rec)
    c.Request, _ = http.NewRequest("GET", "/fail1", nil)

    failStatus(c)

    res := rec.Result()
    if res.StatusCode != http.StatusNotFound { // Should be 404
        t.Fatalf("Expecting status to be 404 got %v", res.StatusCode)
    }
}

func TestFailJson(t *testing.T) {
    gin.SetMode(gin.TestMode)

    rec := httptest.NewRecorder()
    c, _ := gin.CreateTestContext(rec)
    c.Request, _ = http.NewRequest("GET", "/fail1", nil)

    failJson(c)

    res := rec.Result()
    if res.StatusCode != http.StatusNotFound { // Should be 404
        t.Fatalf("Expecting status to be 404 got %v", res.StatusCode)
    }
}
easonlin404 commented 7 years ago

Because testing handler didn't through gin.Engine(router). You can use the following example:

package main

import (
    "net/http"
    "net/http/httptest"
    "testing"

    "github.com/gin-gonic/gin"
)

func TestFailStatus(t *testing.T) {
    gin.SetMode(gin.TestMode)

    router := gin.New()

    res:=performRequest("GET","/fail1",router)

    if res.Code != http.StatusNotFound { // Should be 404
        t.Fatalf("Expecting status to be 404 got %v", res.Code)
    }
}

func TestFailJson(t *testing.T) {
    gin.SetMode(gin.TestMode)

    router := gin.New()

    res:=performRequest("GET","/fail2",router)

    if res.Code != http.StatusNotFound { // Should be 404
        t.Fatalf("Expecting status to be 404 got %v", res.Code)
    }
}

func performRequest(method, target string, router *gin.Engine) *httptest.ResponseRecorder {
    r := httptest.NewRequest(method, target, nil)
    w := httptest.NewRecorder()
    router.ServeHTTP(w, r)
    return w
}
emiguens commented 7 years ago

Hi @easonlin404, thanks for your answer.

Correct me if I'm wrong, but in this case I would be testing the handler and the engine and not only the handler.

In a real project I would have a struct that it's instantiated by injecting dependencies (when testing mocks) and then the a Handler that is called. What I want to test is the logic of my handler, and not the routing/engine/middlewares/handler combination (that would be another kind of test I think).

Either way, the thing that I consider a bug is that c.Status does not behave the same way as c.JSON or c.String in a testing context. The expected behavior would be for all methods of context that assign a status code on the response to work the same way and make the status visible on the httptest.NewRecorder instance.

damianraffa commented 6 years ago

I got the same issue error in testing context.

jbpratt commented 5 years ago

Bumping as error is still reproducible and discussion has been silent. :smile: Any update on the behavior of c.JSON vs c.Status not performing consistently?

lsattem commented 5 years ago

This bug is still present. Setting c.Status(204) does not result in the responserecorder's code being updated. My workaround at the moment is just getting the status from c.Writer.Status() in tests.

marcoaltiericoupa commented 4 years ago

This bug is still present. Setting c.Status(204) does not result in the responserecorder's code being updated. My workaround at the moment is just getting the status from c.Writer.Status() in tests.

This, I think, is the best workaround available.

Alechan commented 3 years ago

Another option is to "force" a flush in the tests with a ginContext.Writer.WriteHeaderNow(). In the example from the first comment:

func TestFailStatus(t *testing.T) {
    gin.SetMode(gin.TestMode)

    rec := httptest.NewRecorder()
    c, _ := gin.CreateTestContext(rec)
    c.Request, _ = http.NewRequest("GET", "/fail1", nil)

    failStatus(c)

    //NEW LINE: Force flush
    c.Writer.WriteHeaderNow()

    res := rec.Result()
    if res.StatusCode != http.StatusNotFound { // Should be 404
        t.Fatalf("Expecting status to be 404 got %v", res.StatusCode)
    }
}
khurram-s commented 3 years ago

Do we have a plan to solve this ? w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Status(204)

w.Result().StatusCode will always return 200.

Thank you for workaround propose above to use c.Writer.Status()

janjulienn-kumu commented 2 years ago

After 5 years since the issue opened, the same issue still occurs.

Using workaround mentioned above. (c.Writer.Status())

j-h-a commented 1 year ago

Still having this problem... did a workaround in the tests like this:

if ctx.Writer.Status() != res.Code {
    ctx.Writer.WriteHeaderNow()
}
LombardiDaniel commented 8 months ago

Issue still happing in 2024...

LombardiDaniel commented 8 months ago

I think I found the [solution] (https://stackoverflow.com/questions/77951149/error-on-testing-gingko-and-gomega-for-gingonic-w-code-not-working/77957924#77957924)

It seems that the httptest.ResponseRecorder can only be written once. In my test-suite, the first test is expected to return 200 OK, so it sets that value in the response recorder.

This seemed to fix the issue:

req, err := http.NewRequest(http.MethodPut, "/v1/create-user", bytes.NewReader(body))
Expect(err).NotTo(HaveOccurred())
req.Header.Set("Content-Type", "application/json")

w := httptest.NewRecorder()  // <- creates a new response recorder

router.ServeHTTP(w, req)
Ω(w.Code).Should(Equal(http.StatusBadGateway))

This way the test runs successfully.

Note that i am NOT using gin.CreateTestContext().