0

I am writing test for an elasticsearch middleware, where I am using a function to build test servers in which I pass a slice of configuration structs for each tests and in a handler function they are iterated upon and the expected response is written to the response writer. This is my function.

func newMockClient(url string) (*elasticsearch, error) {
    client, err := elastic.NewSimpleClient(elastic.SetURL(url))
    if err != nil {
        return nil, fmt.Errorf("error while initializing elastic client: %v", err)
    }
    es := &elasticsearch{
        url:    url,
        client: client,
    }
    return es, nil
}

type ServerSetup struct {
    Method, Path, Body, Response string
    HTTPStatus                   int
}

func buildTestServer(t *testing.T, setups []*ServerSetup) *httptest.Server {
    handlerFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        requestBytes, _ := ioutil.ReadAll(r.Body)
        requestBody := string(requestBytes)

        matched := false
        for _, setup := range setups {
            if r.Method == setup.Method && r.URL.EscapedPath() == setup.Path {
                matched = true
                if setup.HTTPStatus == 0 {
                    w.WriteHeader(http.StatusOK)
                } else {
                    w.WriteHeader(setup.HTTPStatus)
                }
                _, err := w.Write([]byte(setup.Response))
                if err != nil {
                    t.Fatalf("Unable to write test server response: %v", err)
                }
            }
        }

        if !matched {
            t.Fatalf("No requests matched setup. Got method %s, Path %s, body %s", r.Method, r.URL.EscapedPath(), requestBody)
        }
    })

    return httptest.NewServer(handlerFunc)
}

It is copied from github.com/github/vulcanizer. When I run a single test using this, it works fine. For e.g. this test

func TestCreateIndex(t *testing.T) {
    setup := &ServerSetup{
        Method:   "PUT",
        Path:     "/test",
        Response: `{"acknowledged": true, "shards_acknowledged": true, "index": "test"}`,
    }

    ts := buildTestServer(t, []*ServerSetup{setup})

    es, _ := newMockClient(ts.URL)

    err := es.createIndex(context.Background(), "test", nil)
    if err != nil {
        t.Fatalf("Index creation failed with error: %v\n", err)
    }

}

But when I try to check different behaviours in a single test like this one I get an error http: multiple response.WriteHeader calls

func TestDeleteIndex(t *testing.T) {
    setup := &ServerSetup{
        Method:   "DELETE",
        Path:     "/test",
        Response: `{"acknowledged": true}`,
    }

    errSetup := &ServerSetup{
        Method:   "DELETE",
        Path:     "/test",
        Response: `{"acknowledged": false}`,
    }

    ctx := context.Background()

    ts := buildTestServer(t, []*ServerSetup{setup, errSetup})
    defer ts.Close()

    es, _ := newMockClient(ts.URL)

    err := es.deleteIndex(ctx, "test")
    if err != nil {
        t.Fatalf("Index creation failed with error: %v\n", err)
    }

    err = es.deleteIndex(ctx, "test")
    if err == nil {
        t.Fatal("Expected error but not found")
    }
}

I am guessing that it is because of the fact that when I run deleteIndex for the second time it pings the server again but the response writer has already been written to so it can't write anything else to it.

Is there anyway I can have a check at the beginning of my handler func like

handlerFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    if w != nil{
        // clear data in response writer
    }

// .........



}
Palash Nigam
  • 1,653
  • 3
  • 14
  • 26
  • Possible duplicate of [Golang HTTP Check If ResponseWriter Has Been Written](https://stackoverflow.com/questions/39415827/golang-http-check-if-responsewriter-has-been-written) – Jonathan Hall Mar 10 '19 at 18:42
  • 1
    No, there is no way to remove data from a responsewriter. But that doesn't seem to be what you're asking for anyway. – Jonathan Hall Mar 10 '19 at 18:48
  • Where is `newMockClient` defined? Can you include the source for it? – Henry Woody Mar 10 '19 at 19:00
  • 1
    @HenryWoody the function just takes the test server url and returns an elasticsearch client pointing to that. I have updated the code. – Palash Nigam Mar 10 '19 at 19:19
  • Seems like you're reusing the `Request` object for multiple test requests. They are not safe to reuse. – Adrian Mar 11 '19 at 14:53

2 Answers2

3

I don't think what you are doing is the correct way to test your functionality. You need to separate your test to test-cases for checking different behaviours like that:

func Test_DeleteIndex(t *testing.T) {
    t.Run("Should be ok with correct setup", func(t *testing.T) {
        setup := &ServerSetup{
            Method:   "DELETE",
            Path:     "/test",
            Response: `{"acknowledged": true}`,
        }
        ctx := context.Background()
        ts := buildTestServer(t, []*ServerSetup{setup})
        defer ts.Close()
        es, _ := newMockClient(ts.URL)
        err := es.deleteIndex(ctx, "test")
        require.NoError(t, err)
    })

    t.Run("Shouldn't be ok with wrong setup", func(t *testing.T) {
        setup := &ServerSetup{
            Method:   "DELETE",
            Path:     "/test",
            Response: `{"acknowledged": false}`,
        }
        ctx := context.Background()
        ts := buildTestServer(t, []*ServerSetup{setup})
        defer ts.Close()
        es, _ := newMockClient(ts.URL)
        err := es.deleteIndex(ctx, "test")
        require.Error(t, err)
    })
}
Igor Tulmentyev
  • 341
  • 2
  • 10
2

The issue here is that for each request that the test server gets, the handler loops over all the ServerSetup structs checking for matches based on method and path, but does not break out of the loop upon finding a match.

So in your second test case, since you pass two setup structs with the same Method and Path, two setup cases will match an incoming request for DELETE /test, and the program will try to call WriteHeader on the ResponseWriter twice.

There are two ways I can think of to resolve this issue:

Option 1

If you want to have the server respond differently to successive calls of the same method and path combination, you can add an attribute to check if the ServerSetup instance has been used already, and avoid any setup structs that have already been used.

For example:

type ServerSetup struct {
    Method, Path, Body, Response    string
    HTTPStatus                      int
    HasBeenCalled                   bool
}

func buildTestServer(t *testing.T, setups []*ServerSetup) *httptest.Server {
    handlerFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        requestBytes, _ := ioutil.ReadAll(r.Body)
        requestBody := string(requestBytes)

        matched := false
        for _, setup := range setups {
            if setup.HasBeenCalled {
                continue // skip those that have already been called
            }
            if r.Method == setup.Method && r.URL.EscapedPath() == setup.Path {
                setup.HasBeenCalled = true
                matched = true
                if setup.HTTPStatus == 0 {
                    w.WriteHeader(http.StatusOK)
                } else {
                    w.WriteHeader(setup.HTTPStatus)
                }
                _, err := w.Write([]byte(setup.Response))
                if err != nil {
                    t.Fatalf("Unable to write test server response: %v", err)
                }
            }
            if matched {
                break // stop checking for matches if already found match
            }
        }

        if !matched {
            t.Fatalf("No requests matched setup. Got method %s, Path %s, body %s", r.Method, r.URL.EscapedPath(), requestBody)
        }
    })

    return httptest.NewServer(handlerFunc)
}

Option 2

A slightly simpler way to resolve this issue would be to create separate test servers for each of these two cases, one for each setup struct, since they involve different results from the same method-path combination. More cleanly, you could separate these into two separate tests.

So you would end up with:

func TestDeleteIndex_Success(t *testing.T) {
    setup := &ServerSetup{
        Method:   "DELETE",
        Path:     "/test",
        Response: `{"acknowledged": true}`,
    }

    ctx := context.Background()

    ts := buildTestServer(t, []*ServerSetup{setup})
    defer ts.Close()

    es, _ := newMockClient(ts.URL)

    err := es.deleteIndex(ctx, "test")
    if err != nil {
        t.Fatalf("Index creation failed with error: %v\n", err)
    }
}

func TestDeleteIndex_Error(t *testing.T) {
    errSetup := &ServerSetup{
        Method:   "DELETE",
        Path:     "/test",
        Response: `{"acknowledged": false}`,
    }

    ctx := context.Background()

    ts := buildTestServer(t, []*ServerSetup{errSetup})
    defer ts.Close()

    es, _ := newMockClient(ts.URL)

    err := es.deleteIndex(ctx, "test")
    if err == nil {
        t.Fatal("Expected error but not found")
    }
}

You should also in the future avoid passing multiple ServerSetup structs with the same method-path combo in the future to avoid this error.

Henry Woody
  • 14,024
  • 7
  • 39
  • 56