steinfletcher/apitest

How to set RemoteAddr?

kierun opened this issue · 4 comments

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")
}

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.

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 an update 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.

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.

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.