Description

ctx.Status(http.StatusCreated) is not setting up status in response and I am always getting 200. But If I add ctx.Writer.WriteHeaderNow() with ctx.Status() then it is working fine.

How to reproduce

// Sample Function
func PostCategory(ctx *gin.Context) {
    ctx.Status(http.StatusCreated)
}

// Sample Test Function
func TestPostCategory(t *testing.T) {
    r := httptest.NewRequest(http.MethodGet, "/", nil)
    w := httptest.NewRecorder()
    ctx, _ := gin.CreateTestContext(w)
    ctx.Request = r

    PostCategory(ctx)
    assert.Equal(t, http.StatusCreated, w.Code)
}

Expectations

We must get 201 status code and test must be passed.

Actual result

We are getting 200 status code in response and test are failing

Environment

  • go version: go1.18.8 linux/amd64
  • gin version: v1.8.1
  • operating system: Ubuntu 18.04 LTS

Comment From: webstradev

I'm not sure this is the answer that you are looking for, but I don't think this is a bug.

When unit testing gin handlers you will likely want to actually register the route on a gin engine and serve the request.

Consider using the gin.Engine that is returned from the gin.CreateTextContext(w) function.

Your code would then look something like this

// Sample Function
func PostCategory(ctx *gin.Context) {
    ctx.Status(http.StatusCreated)
}

// Sample Test Function
func TestPostCategory(t *testing.T) {
    r := httptest.NewRequest(http.MethodGet, "/", nil)
    w := httptest.NewRecorder()
    _, router := gin.CreateTestContext(w) // I've changed this line to use the gin engine

    router.GET("/", PostCategory) // I've added this line to register a route using the function you want to test

    router.ServeHTTP(w, r) // I've added this line to actually serve the request to the router we've defined above

    assert.Equal(t, http.StatusCreated, w.Code)
}

You can find another example of this in the gin documentation

Let me know if that helps 😄

Comment From: infected-engineer

I'm still considering it as bug because if you execute the given example in issue's description with standard library without using gin then you will status set in w.Code.

The reason is because gin is not setting up status in w.Code while using httptest.NewRecorder() It should set status in httptest.NewRecorder() as well as in gin's status

Comment From: webstradev

I'll leave this for one of the maintainers do decide on as I'm not familiar enough with the project to know if this is behaving as intended or not.

Comment From: oantoro

I guess it is because gin implements ResponseWriter.WriteHeader() differently from net/http's implementation. After calling .WriteHeader() from ctx.Status(), gin defers sending the status code until .WriteHeaderNow() is called. .WriteHeaderNow() will be called after appropriate handlers are called in engine.handleHTTPRequest(). https://github.com/gin-gonic/gin/blob/0c96a20209ca035964be126a745c167196fb6db3/gin.go#L617 That code is never executed in the example in the issue description above. I am afraid that changing the behavior of ctx.Status() will change the way gin's response flow works that leads to breaking changes. The solution to this issue is as suggested by @webstradev but line PostCategory(ctx) is not needed as the function PostCategory() is already called by gin. But, the different behavior of .WriteHeader() between gin and net/http would be interesting to discuss.

Comment From: webstradev

I guess it is because gin implements ResponseWriter.WriteHeader() differently from net/http's implementation. After calling .WriteHeader() from ctx.Status(), gin defers sending the status code until .WriteHeaderNow() is called. .WriteHeaderNow() will be called after appropriate handlers are called in engine.handleHTTPRequest().

https://github.com/gin-gonic/gin/blob/0c96a20209ca035964be126a745c167196fb6db3/gin.go#L617

That code is never executed in the example in the issue description above. I am afraid that changing the behavior of ctx.Status() will change the way gin's response flow works that leads to breaking changes. The solution to this issue is as suggested by @webstradev but line PostCategory(ctx) is not needed as the function PostCategory() is already called by gin. But, the different behavior of .WriteHeader() between gin and net/http would be interesting to discuss.

Have amended my suggestion to remove the line your referenced

Comment From: alexgille

Hello,

I'm facing the same issue with Status() while unit testing. It would be great of this discussion could come to a final decision.

Let me know if I can help.

Regards

Comment From: gragorther

I'm having the same issue, here's my handler:

func RegisterUser(db interface {
    CheckIfUserExistsByUsernameAndEmail(username string, email string) (bool, error)
    CreateUser(*models.User) error
}) gin.HandlerFunc {
    return func(c *gin.Context) {

        var authInput RegistrationInput

        if err := c.ShouldBindJSON(&authInput); err != nil {
            c.AbortWithError(http.StatusUnprocessableEntity, fmt.Errorf("failed to bind register user JSON: %w", err))
            return
        }
        validEmail := email.Validate(authInput.Email)
        if !validEmail {
            c.AbortWithStatus(http.StatusUnprocessableEntity)
            return
        }

        userExists, err := db.CheckIfUserExistsByUsernameAndEmail(authInput.Username, authInput.Email)

        if err != nil {
            c.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to register user: %w", err))
            return
        }
        if userExists {
            c.AbortWithStatus(http.StatusConflict)
            return
        }

        passwordHash, err := argon2id.CreateHash(authInput.Password, argon2id.DefaultParams)
        if err != nil {
            c.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to register user: %w", err))
            return
        }

        user := models.User{
            Username:     authInput.Username,
            PasswordHash: string(passwordHash),
            Email:        authInput.Email,
            Name:         authInput.Name,
        }

        if err := db.CreateUser(&user); err != nil {
            c.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to create user: %w", err))
            return
        }

        c.Status(http.StatusCreated)
    }
}

And the (shortened) test for it:

func TestRegisterUser(t *testing.T) {
    t.Run("valid input", func(t *testing.T) {
        c, w, assert := setupHandlerTest(t)
        mock := newMockDB()
        name := "Down"
        input, err := sonic.Marshal(handlers.RegistrationInput{
            Username: "mark",
            Name:     &name,
            Email:    "test@google.com",
            Password: "5UP3RS3CR37",
        })
        if err != nil {
            t.Fatalf("sonic failed to bind json, %v", err)
        }
        setGinHttpBody(c, input)

        handlers.RegisterUser(mock)(c)

        assert.Equal(http.StatusCreated, w.Code, "http status code should indicate that the user was created")
    })
}

The test does not pass:

--- FAIL: TestRegisterUser (0.08s)
    --- FAIL: TestRegisterUser/valid_input (0.08s)
        user_test.go:44: 
                Error Trace:    /home/gregor/epigo/handlers/user_test.go:44
                Error:          Not equal: 
                                expected: 201
                                actual  : 200
                Test:           TestRegisterUser/valid_input
                Messages:       http status code should indicate that the user was created

The handler didn't fail, and for some reason it returns a 200, but the test does pass if I use c.AbortWithStatus(http.StatusCreated)