steinfletcher / apitest

A simple and extensible behavioural testing library for Go. You can use api test to simplify REST API, HTTP handler and e2e tests.
https://apitest.dev
MIT License
795 stars 55 forks source link

How to set RemoteAddr? #111

Closed kierun closed 3 years ago

kierun commented 3 years ago

Is your feature request related to a problem? Please describe.

I have a simple function which finds out which IP address has made a request. It checks first X-REAL-IP then X-FORWARDED-FOR then falls back to RemoteAddr. I know that's not accurate but, in lieu of anything better, it will have to do.

Describe the solution you'd like

I cannot seem to test the code that does RemoteAddr as I am unable to set it within the apitest.New()[…] chain.

I might have missed it in the documentation.

Describe alternatives you've considered

Not testing it is fine really (the code is hardly complex and not massively critical) but I am aiming for 100% coverage…

Additional context

Here is the get IP code, if that makes any difference?

func getIP(r *http.Request) (string, error) {

        // Get IP from the X-REAL-IP header
        ip := r.Header.Get("X-REAL-IP")
        netIP := net.ParseIP(ip)
        if netIP != nil {
                return ip, nil
        }

        // Get IP from X-FORWARDED-FOR header
        ips := r.Header.Get("X-FORWARDED-FOR")
        splitIps := strings.Split(ips, ",")
        for _, ip := range splitIps {
                netIP := net.ParseIP(ip)
                if netIP != nil {
                        return ip, nil
                }
        }

        // Get IP from RemoteAddr
        ip, _, err := net.SplitHostPort(r.RemoteAddr)
        if err != nil {
                return "", err
        }
        netIP = net.ParseIP(ip)
        if netIP != nil {
                return ip, nil
        }

        // Bad!
        return "", fmt.Errorf("No valid ip found")
}
steinfletcher commented 3 years ago

Hi @kierun, sorry for the late reply, I completely missed this. There is also a slack channel on Gopherslack which is useful for getting quicker replies.

Is there anything observable from the output of the getIP function you are testing, e.g. do you pass the result onto a mock or return it in the response? What do you do with the result? If the result isn't exposed anywhere then it's difficult to test because we treat the app as a black box - we test the observable behaviour not the internal implementation.

I am wondering if a simple unit test of that code would be better.

kierun commented 3 years ago

No worries whatsoever for the late reply. It's all fine/

I use tests like this:

    apitest.New().
        HandlerFunc(env.handleUpdateCompany).
        Post("/api/v1/company").
        JSON(update.ToJSON()).
        Header("X-FORWARDED-FOR", "::1").
        Expect(t).
        Status(http.StatusInternalServerError).
        End()

The env.handleUpdateCompan does store the IP addresses where the request is coming from. So, I am testing it via adding Header()". It gets saved in anupdate structto a database viagorm`. I can inspect that update (as it is passed to a mock in test and not a real dataase) and check it is there.

I guess I could just write a simple unit test for RemoteAddr without going through the REST API. However, I was wondering if there was a simpler way to deal with it.

steinfletcher commented 3 years ago

Hey @kierun, yes I think unit test for RemoteAddr seems sensible and a lot simpler than checking results in the DB.

An option could be to inject a mock for the DB layer into your handler, then you can use API test still and check the mock result. I think that's a fairly common pattern but depends on dependency injection setup within your app.

kierun commented 3 years ago

I tend to prefer simple to complicated. I shall unit test it in isolation. Thank you.

I have been really enjoying using apitest. It's really nice to work with. Thank you for your hard work.