Change error handling and update all tests

main_test.go

@@ -4,27 +4,31 @@ import (
 	"testing"
 
 	"github.com/ryanehamil/lookupip/src/ipapi"
-	"github.com/ryanehamil/lookupip/src/utils"
 )
 
 // TestBuildURL
 // Tests that the buildURL function returns a correct URL
-func TestBuildURL(t *testing.T) {
+func TestLookup(t *testing.T) {
 	var tests = []struct {
 		input   string
 		want    string
 		explain string
 	}{
-		{"nothing", "any ip address", "Valid Return"},
+		{"", "Any IPV4", "Lookup my own IP"},
+		{"8.8.8.8", "8.8.8.8", "Lookup Google's DNS"},
 	}
 
 	for _, test := range tests {
+		ip := test.input
+		properties := ""
 		// Use the IP-API to lookup anything
-		got := ipapi.Lookup(ip, properties).String()
+		data, _ := ipapi.Lookup(&ip, &properties)
-		// use regex to check if got is a valid ipv4 address
 
-		if !utils.CheckValidIP(got) {
+		got := ipapi.GetProperties(data, properties, detail)
-			t.Errorf("ipapi.Lookup returned %s using input %s, want %s", got, test.input, test.want)
+
+		if got != test.want {
+			t.Errorf("%q. lookup(%q) = %v, want %v", test.explain, test.input, got, test.want)
 		}
+
 	}
 }

src/ipapi/ip_api.go

@@ -2,6 +2,8 @@ package ipapi
 
 import (
 	"encoding/json"
+	"errors"
+	"fmt"
 	"net/http"
 	"reflect"
 	"strings"
@@ -43,44 +45,52 @@ func (i *IPAPI) String() string {
 
 // Build URL to query IP-API
 // https://ip-api.com/docs/api:json
-func buildURL(ip string) string {
+func buildURL(ip string) (string, error) {
-	return "http://ip-api.com/json/" + ip
+	url := "http://ip-api.com/json"
+	if ip != "" {
+		ip = strings.TrimSpace(ip)
+		if !utils.CheckValidIP(ip) {
+			var err = errors.New("invalid IP address, failed CheckValidIP")
+			return url, err
+		}
+		url += fmt.Sprintf("/%s", ip)
+	}
+	return url, nil
 }
 
 // Get IP-API data about IP
 //
 // https://ip-api.com/docs/api:json
-func Lookup(ip string, properties string) *IPAPI {
+func Lookup(ip *string, properties *string) (data *IPAPI, err error) {
-	var data *IPAPI
-
-	if ip == "" {
-		if properties == "" {
-			properties = "Query"
-		}
-	} else {
-		if properties == "" {
-			properties = "Country"
-		}
-		if !utils.CheckValidIP(ip) {
-			utils.PrintOut("Invalid IP address during Lookup")
-			utils.Exit(1)
-		}
 
+	if *ip == "" && *properties == "" {
+		*properties = "Query"
+	} else if *properties == "" {
+		*properties = "Country"
 	}
-	url := buildURL(ip)
+
+	url, error := buildURL(*ip)
+	if error != nil {
+		return data, error
+	}
+
 	resp, err := http.Get(url)
-	utils.HandleError(err)
+	if error != nil {
+		return data, error
+	}
 
 	defer resp.Body.Close()
 
 	err = json.NewDecoder(resp.Body).Decode(&data)
-	utils.HandleError(err)
+	if error != nil {
+		return data, error
+	}
 
 	if data.Status == "fail" {
 		utils.PrintOut("IP-API returned an error: " + data.Message)
 		utils.Exit(1)
 	}
-	return data
+	return data, nil
 }
 
 func GetProperties(data *IPAPI, properties_string string, detail bool) string {
@@ -93,8 +103,9 @@ func GetProperties(data *IPAPI, properties_string string, detail bool) string {
 		if datafield != "" {
 			result = datafield
 		} else {
-			result = "Not found"
+			result = ""
 		}
+
 		if detail {
 			output += property + ": " + result + "\n"
 		} else {

src/ipapi/ip_api_test.go

@@ -2,6 +2,8 @@ package ipapi
 
 import (
 	"testing"
+
+	"github.com/ryanehamil/lookupip/src/utils"
 )
 
 // TestBuildURL
@@ -12,13 +14,20 @@ func TestBuildURL(t *testing.T) {
 		want    string
 		explain string
 	}{
-		{"127.0.0.1", "http://ip-api.com/json/127.0.0.1", "Valid Return"},
+		{"127.0.0.1", "http://ip-api.com/json/127.0.0.1", "Valid IP"},
+		{"  127.0.0.1 ", "http://ip-api.com/json/127.0.0.1", "Dirty IP"},
+		{"127.foo.bar.1", "invalid IP address, failed CheckValidIP", "Unsantized input"},
 	}
 
 	for _, test := range tests {
-		got := buildURL(test.ip)
+		got, err := buildURL(test.ip)
-		if got != test.want {
+
-			t.Errorf("CheckValidIP(%q) = %v, want %v", test.ip, got, test.want)
+		if err != nil {
+			if err.Error() != test.want {
+				t.Errorf("%q. error-buildURL(%q) = %v, want %v", test.explain, test.ip, got, test.want)
+			}
+		} else if got != test.want {
+			t.Errorf("%q. buildURL(%q) = %v, want %v", test.explain, test.ip, got, test.want)
 		}
 	}
 }
@@ -31,14 +40,49 @@ func TestLookup(t *testing.T) {
 		want    string
 		explain string
 	}{
-		{"8.8.8.8", "United States", "Valid Return"},
+		{"8.8.8.8", "United States", "Google Public DNS"},
+		{"199.211.133.90", "United States", "Naval Oceanography"},
+		{"116.202.3.251", "Germany", "JAM Software Germany"},
+		{"", "Any IP", "Get my IP"},
 	}
 
 	for _, test := range tests {
-		got := Lookup(test.ip, "")
+		properties := ""
-		if test.want != got.Country {
+		data, err := Lookup(&test.ip, &properties)
-			t.Errorf("CheckValidIP(%q) = %v, want %v", test.ip, got, test.want)
+		got := data.Country
+		if err != nil && err.Error() != test.want {
+			t.Errorf("%q. error-buildURL(%q) = %v, want %v", test.explain, test.ip, got, test.want)
+		} else if test.want == "Any IP" {
+			if !utils.CheckValidIP(data.Query) {
+				t.Errorf("%q. error-buildURL(%q) = %v, want %v", test.explain, test.ip, got, test.want)
+			}
+		} else if got != test.want {
+			t.Errorf("%q. buildURL(%q) = %v, want %v", test.explain, test.ip, got, test.want)
 		}
 
 	}
 }
+
+func TestGetProperties(t *testing.T) {
+	var tests = []struct {
+		properties string
+		want       string
+		explain    string
+	}{
+		{"", "United States", "No properties requested"},
+		{"Country", "United States", "Properties: Country"},
+		{"Country,ISP", "United States,Google LLC", "Properties: Country,ISP"},
+	}
+
+	for _, test := range tests {
+		// This test relies on the lookup function
+		ip := "8.8.8.8"
+		// Use the IP-API to lookup anything
+		data, _ := Lookup(&ip, &test.properties)
+
+		got := GetProperties(data, test.properties, false)
+		if got != test.want {
+			t.Errorf("%q. GetProperties(%q) = %v, want %v", test.explain, test.properties, got, test.want)
+		}
+	}
+}

src/utils/utils_test.go

@@ -12,13 +12,13 @@ func TestCheckValidIP(t *testing.T) {
 		explain string
 	}{
 		{"127.0.0.1", true, "Valid IP"},
-		{"127.O.O.1", false, "Invalid IP"},
+		{"127.X.X.1", false, "Invalid IP"},
 	}
 
 	for _, test := range tests {
 		got := CheckValidIP(test.ip)
 		if got != test.want {
-			t.Errorf("CheckValidIP(%q) = %v, want %v", test.ip, got, test.want)
+			t.Errorf("%q. CheckValidIP(%q) = %v", test.explain, test.ip, got)
 		}
 	}
 }