Refactor TestParseURL

This is in preparation for supporting query parameters
in ParseURL:

- use an expected *Options instance to execute assertions on
- extract assertions into helper function
- enable parallel testing
- condense test table

options_test.go

@@ -4,6 +4,7 @@
 package redis
 
 import (
+	"crypto/tls"
 	"errors"
 	"testing"
 	"time"
@@ -11,150 +12,108 @@ import (
 
 func TestParseURL(t *testing.T) {
 	cases := []struct {
-		u    string
+		url string
-		addr string
+		o   *Options // expected value
-		db   int
+		err error
-		tls  bool
-		err  error
-		user string
-		pass string
 	}{
 		{
-			"redis://localhost:123/1",
+			url: "redis://localhost:123/1",
-			"localhost:123",
+			o:   &Options{Addr: "localhost:123", DB: 1},
-			1, false, nil,
+		}, {
-			"", "",
+			url: "redis://localhost:123",
-		},
+			o:   &Options{Addr: "localhost:123"},
-		{
+		}, {
-			"redis://localhost:123",
+			url: "redis://localhost/1",
-			"localhost:123",
+			o:   &Options{Addr: "localhost:6379", DB: 1},
-			0, false, nil,
+		}, {
-			"", "",
+			url: "redis://12345",
-		},
+			o:   &Options{Addr: "12345:6379"},
-		{
+		}, {
-			"redis://localhost/1",
+			url: "rediss://localhost:123",
-			"localhost:6379",
+			o:   &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ /* no deep comparison */ }},
-			1, false, nil,
+		}, {
-			"", "",
+			url: "redis://:bar@localhost:123",
-		},
+			o:   &Options{Addr: "localhost:123", Password: "bar"},
-		{
+		}, {
-			"redis://12345",
+			url: "redis://foo@localhost:123",
-			"12345:6379",
+			o:   &Options{Addr: "localhost:123", Username: "foo"},
-			0, false, nil,
+		}, {
-			"", "",
+			url: "redis://foo:bar@localhost:123",
-		},
+			o:   &Options{Addr: "localhost:123", Username: "foo", Password: "bar"},
-		{
+		}, {
-			"rediss://localhost:123",
+			url: "unix:///tmp/redis.sock",
-			"localhost:123",
+			o:   &Options{Addr: "/tmp/redis.sock"},
-			0, true, nil,
+		}, {
-			"", "",
+			url: "unix://foo:bar@/tmp/redis.sock",
-		},
+			o:   &Options{Addr: "/tmp/redis.sock", Username: "foo", Password: "bar"},
-		{
+		}, {
-			"redis://:bar@localhost:123",
+			url: "unix://foo:bar@/tmp/redis.sock?db=3",
-			"localhost:123",
+			o:   &Options{Addr: "/tmp/redis.sock", Username: "foo", Password: "bar", DB: 3},
-			0, false, nil,
+		}, {
-			"", "bar",
+			url: "unix://foo:bar@/tmp/redis.sock?db=test",
-		},
+			err: errors.New(`redis: invalid database number: strconv.Atoi: parsing "test": invalid syntax`),
-		{
+		}, {
-			"redis://foo@localhost:123",
+			url: "redis://localhost/?abc=123",
-			"localhost:123",
+			err: errors.New("redis: no options supported"),
-			0, false, nil,
+		}, {
-			"foo", "",
+			url: "http://google.com",
-		},
+			err: errors.New("redis: invalid URL scheme: http"),
-		{
+		}, {
-			"redis://foo:bar@localhost:123",
+			url: "redis://localhost/1/2/3/4",
-			"localhost:123",
+			err: errors.New("redis: invalid URL path: /1/2/3/4"),
-			0, false, nil,
+		}, {
-			"foo", "bar",
+			url: "12345",
-		},
+			err: errors.New("redis: invalid URL scheme: "),
-		{
+		}, {
-			"unix:///tmp/redis.sock",
+			url: "redis://localhost/iamadatabase",
-			"/tmp/redis.sock",
+			err: errors.New(`redis: invalid database number: "iamadatabase"`),
-			0, false, nil,
-			"", "",
-		},
-		{
-			"unix://foo:bar@/tmp/redis.sock",
-			"/tmp/redis.sock",
-			0, false, nil,
-			"foo", "bar",
-		},
-		{
-			"unix://foo:bar@/tmp/redis.sock?db=3",
-			"/tmp/redis.sock",
-			3, false, nil,
-			"foo", "bar",
-		},
-		{
-			"unix://foo:bar@/tmp/redis.sock?db=test",
-			"/tmp/redis.sock",
-			0, false, errors.New("redis: invalid database number: strconv.Atoi: parsing \"test\": invalid syntax"),
-			"", "",
-		},
-		{
-			"redis://localhost/?abc=123",
-			"",
-			0, false, errors.New("redis: no options supported"),
-			"", "",
-		},
-		{
-			"http://google.com",
-			"",
-			0, false, errors.New("redis: invalid URL scheme: http"),
-			"", "",
-		},
-		{
-			"redis://localhost/1/2/3/4",
-			"",
-			0, false, errors.New("redis: invalid URL path: /1/2/3/4"),
-			"", "",
-		},
-		{
-			"12345",
-			"",
-			0, false, errors.New("redis: invalid URL scheme: "),
-			"", "",
-		},
-		{
-			"redis://localhost/iamadatabase",
-			"",
-			0, false, errors.New(`redis: invalid database number: "iamadatabase"`),
-			"", "",
 		},
 	}
 
-	for _, c := range cases {
+	for i := range cases {
-		t.Run(c.u, func(t *testing.T) {
+		tc := cases[i]
-			o, err := ParseURL(c.u)
+		t.Run(tc.url, func(t *testing.T) {
-			if c.err == nil && err != nil {
+			t.Parallel()
+
+			actual, err := ParseURL(tc.url)
+			if tc.err == nil && err != nil {
 				t.Fatalf("unexpected error: %q", err)
 				return
 			}
-			if c.err != nil && err != nil {
+			if tc.err != nil && err != nil {
-				if c.err.Error() != err.Error() {
+				if tc.err.Error() != err.Error() {
-					t.Fatalf("got %q, expected %q", err, c.err)
+					t.Fatalf("got %q, expected %q", err, tc.err)
 				}
 				return
 			}
-			if o.Addr != c.addr {
+			comprareOptions(t, actual, tc.o)
-				t.Errorf("got %q, want %q", o.Addr, c.addr)
-			}
-			if o.DB != c.db {
-				t.Errorf("got %q, expected %q", o.DB, c.db)
-			}
-			if c.tls && o.TLSConfig == nil {
-				t.Errorf("got nil TLSConfig, expected a TLSConfig")
-			}
-			if o.Username != c.user {
-				t.Errorf("got %q, expected %q", o.Username, c.user)
-			}
-			if o.Password != c.pass {
-				t.Errorf("got %q, expected %q", o.Password, c.pass)
-			}
 		})
 	}
 }
 
+func comprareOptions(t *testing.T, actual, expected *Options) {
+	t.Helper()
+
+	if actual.Addr != expected.Addr {
+		t.Errorf("got %q, want %q", actual.Addr, expected.Addr)
+	}
+	if actual.DB != expected.DB {
+		t.Errorf("got %q, expected %q", actual.DB, expected.DB)
+	}
+	if actual.TLSConfig == nil && expected.TLSConfig != nil {
+		t.Errorf("got nil TLSConfig, expected a TLSConfig")
+	}
+	if actual.TLSConfig != nil && expected.TLSConfig == nil {
+		t.Errorf("got TLSConfig, expected no TLSConfig")
+	}
+	if actual.Username != expected.Username {
+		t.Errorf("got %q, expected %q", actual.Username, expected.Username)
+	}
+	if actual.Password != expected.Password {
+		t.Errorf("got %q, expected %q", actual.Password, expected.Password)
+	}
+}
+
 // Test ReadTimeout option initialization, including special values -1 and 0.
 // And also test behaviour of WriteTimeout option, when it is not explicitly set and use
 // ReadTimeout value.