✅ create internal/net/http/client option test (#831)

* fix

* add option test

* revert client test

* fix

* 🤖 Update license headers / Format go codes and yaml files

Signed-off-by: vdaas-ci <ci@vdaas.org>

* add 0 value test

* remove default value definition in test

Co-authored-by: vdaas-ci <ci@vdaas.org>

.github/conflint.yaml

@@ -1,5 +1,5 @@
 kubeval:
   - files:
-    - k8s/**/*.yaml
+      - k8s/**/*.yaml
     strict: true
     ignoreMissingSchemas: true

internal/db/nosql/cassandra/cassandra_mock.go

@@ -1,3 +1,18 @@
+//
+// Copyright (C) 2019-2020 Vdaas.org Vald team ( kpango, rinx, kmrmt )
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
 package cassandra
 
 import "github.com/gocql/gocql"

internal/net/http/client/client.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/vdaas/vald/internal/backoff"
 	"github.com/vdaas/vald/internal/errors"
+	"github.com/vdaas/vald/internal/log"
 	htr "github.com/vdaas/vald/internal/net/http/transport"
 	"golang.org/x/net/http2"
 )
@@ -35,14 +36,19 @@ func New(opts ...Option) (*http.Client, error) {
 	tr := new(transport)
 	tr.Transport = new(http.Transport)
 
-	for _, opt := range append(defaultOptions, opts...) {
+	for _, opt := range append(defaultOpts, opts...) {
 		if err := opt(tr); err != nil {
-			return nil, errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+			werr := errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+			e := new(errors.ErrCriticalOption)
+			if errors.As(err, &e) {
+				log.Error(werr)
+				return nil, werr
+			}
+			log.Warn(werr)
 		}
 	}
 
-	err := http2.ConfigureTransport(tr.Transport)
-	if err != nil {
+	if err := http2.ConfigureTransport(tr.Transport); err != nil {
 		return nil, err
 	}
 

internal/net/http/client/client_test.go

@@ -17,14 +17,53 @@
 package client
 
 import (
+	"context"
+	"net"
 	"net/http"
+	"net/url"
 	"reflect"
+	"sync"
+	"sync/atomic"
 	"testing"
 
+	"github.com/vdaas/vald/internal/backoff"
 	"github.com/vdaas/vald/internal/errors"
+	"github.com/vdaas/vald/internal/test/comparator"
 	"go.uber.org/goleak"
 )
 
+var (
+	// Goroutine leak is detected by `fastime`, but it should be ignored in the test because it is an external package.
+	goleakIgnoreOptions = []goleak.Option{
+		goleak.IgnoreTopFunction("github.com/kpango/fastime.(*Fastime).StartTimerD.func1"),
+	}
+
+	transportComparator = []comparator.Option{
+		comparator.AllowUnexported(transport{}),
+		comparator.AllowUnexported(http.Transport{}),
+		comparator.IgnoreFields(http.Transport{}, "idleLRU"),
+
+		comparator.Comparer(func(x, y backoff.Option) bool {
+			return reflect.ValueOf(x).Pointer() == reflect.ValueOf(y).Pointer()
+		}),
+		comparator.Comparer(func(x, y func(*http.Request) (*url.URL, error)) bool {
+			return reflect.ValueOf(x).Pointer() == reflect.ValueOf(y).Pointer()
+		}),
+		comparator.Comparer(func(x, y func(ctx context.Context, network, addr string) (net.Conn, error)) bool {
+			return reflect.ValueOf(x).Pointer() == reflect.ValueOf(y).Pointer()
+		}),
+		comparator.Comparer(func(x, y sync.Mutex) bool {
+			return reflect.DeepEqual(x, y)
+		}),
+		comparator.Comparer(func(x, y atomic.Value) bool {
+			return reflect.DeepEqual(x.Load(), y.Load())
+		}),
+		comparator.Comparer(func(x, y sync.Once) bool {
+			return reflect.DeepEqual(x, y)
+		}),
+	}
+)
+
 func TestNew(t *testing.T) {
 	type args struct {
 		opts []Option
@@ -62,7 +101,6 @@ func TestNew(t *testing.T) {
 		       checkFunc: defaultCheckFunc,
 		   },
 		*/
-
 		// TODO test cases
 		/*
 		   func() test {
@@ -80,7 +118,7 @@ func TestNew(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(tt *testing.T) {
-			defer goleak.VerifyNone(tt)
+			defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
 			if test.beforeFunc != nil {
 				test.beforeFunc(test.args)
 			}

internal/net/http/client/option.go

@@ -23,45 +23,55 @@ import (
 	"net/url"
 
 	"github.com/vdaas/vald/internal/backoff"
+	"github.com/vdaas/vald/internal/errors"
 	"github.com/vdaas/vald/internal/timeutil"
 )
 
+// Option represent the functional option for transport.
 type Option func(*transport) error
 
 var (
-	defaultOptions = []Option{
+	defaultOpts = []Option{
 		WithProxy(http.ProxyFromEnvironment),
 		WithEnableKeepAlives(true),
 		WithEnableCompression(true),
 	}
 )
 
+// WithProxy returns the option to set the transport proxy.
 func WithProxy(px func(*http.Request) (*url.URL, error)) Option {
 	return func(tr *transport) error {
-		if px != nil {
-			tr.Proxy = px
+		if px == nil {
+			return errors.NewErrInvalidOption("proxy", px)
 		}
+		tr.Proxy = px
 
 		return nil
 	}
 }
 
+// WithDialContext returns the option to set the dial context.
 func WithDialContext(dx func(ctx context.Context, network, addr string) (net.Conn, error)) Option {
 	return func(tr *transport) error {
-		if dx != nil {
-			tr.DialContext = dx
+		if dx == nil {
+			return errors.NewErrInvalidOption("dialContext", dx)
 		}
+		tr.DialContext = dx
 
 		return nil
 	}
 
 }
 
+// WithTLSHandshakeTimeout returns the option to set the TLS handshake timeout.
 func WithTLSHandshakeTimeout(dur string) Option {
 	return func(tr *transport) error {
+		if len(dur) == 0 {
+			return errors.NewErrInvalidOption("TLSHandshakeTimeout", dur)
+		}
 		d, err := timeutil.Parse(dur)
 		if err != nil {
-			return err
+			return errors.NewErrCriticalOption("TLSHandshakeTimeout", dur, err)
 		}
 
 		tr.TLSHandshakeTimeout = d
@@ -70,6 +80,7 @@ func WithTLSHandshakeTimeout(dur string) Option {
 	}
 }
 
+// WithEnableKeepAlives returns the option to enable keep alive.
 func WithEnableKeepAlives(enable bool) Option {
 	return func(tr *transport) error {
 		tr.DisableKeepAlives = !enable
@@ -78,6 +89,7 @@ func WithEnableKeepAlives(enable bool) Option {
 	}
 }
 
+// WithEnableCompression returns the option to enable compression.
 func WithEnableCompression(enable bool) Option {
 	return func(tr *transport) error {
 		tr.DisableCompression = !enable
@@ -86,6 +98,7 @@ func WithEnableCompression(enable bool) Option {
 	}
 }
 
+// WithMaxIdleConns returns the option to set the max idle connection.
 func WithMaxIdleConns(cn int) Option {
 	return func(tr *transport) error {
 		tr.MaxIdleConns = cn
@@ -94,6 +107,7 @@ func WithMaxIdleConns(cn int) Option {
 	}
 }
 
+// WithMaxIdleConnsPerHost returns the option to set the max idle connection per host.
 func WithMaxIdleConnsPerHost(cn int) Option {
 	return func(tr *transport) error {
 		tr.MaxIdleConnsPerHost = cn
@@ -102,6 +116,7 @@ func WithMaxIdleConnsPerHost(cn int) Option {
 	}
 }
 
+// WithMaxConnsPerHost returns the option to set the max connections per host.
 func WithMaxConnsPerHost(cn int) Option {
 	return func(tr *transport) error {
 		tr.MaxConnsPerHost = cn
@@ -110,11 +125,15 @@ func WithMaxConnsPerHost(cn int) Option {
 	}
 }
 
+// WithIdleConnTimeout returns the option to set the idle connection timeout.
 func WithIdleConnTimeout(dur string) Option {
 	return func(tr *transport) error {
+		if len(dur) == 0 {
+			return errors.NewErrInvalidOption("idleConnTimeout", dur)
+		}
 		d, err := timeutil.Parse(dur)
 		if err != nil {
-			return err
+			return errors.NewErrCriticalOption("idleConnTimeout", dur, err)
 		}
 
 		tr.IdleConnTimeout = d
@@ -123,11 +142,15 @@ func WithIdleConnTimeout(dur string) Option {
 	}
 }
 
+// WithResponseHeaderTimeout returns the option to set the response header timeout.
 func WithResponseHeaderTimeout(dur string) Option {
 	return func(tr *transport) error {
+		if len(dur) == 0 {
+			return errors.NewErrInvalidOption("responseHeaderTimeout", dur)
+		}
 		d, err := timeutil.Parse(dur)
 		if err != nil {
-			return err
+			return errors.NewErrCriticalOption("responseHeaderTimeout", dur, err)
 		}
 
 		tr.ResponseHeaderTimeout = d
@@ -136,11 +159,15 @@ func WithResponseHeaderTimeout(dur string) Option {
 	}
 }
 
+// WithExpectContinueTimeout returns the option to set the expect continue timeout.
 func WithExpectContinueTimeout(dur string) Option {
 	return func(tr *transport) error {
+		if len(dur) == 0 {
+			return errors.NewErrInvalidOption("expectContinueTimeout", dur)
+		}
 		d, err := timeutil.Parse(dur)
 		if err != nil {
-			return err
+			return errors.NewErrCriticalOption("expectContinueTimeout", dur, err)
 		}
 
 		tr.ExpectContinueTimeout = d
@@ -149,56 +176,62 @@ func WithExpectContinueTimeout(dur string) Option {
 	}
 }
 
+//  WithProxyConnectHeader returns the option to set the proxy connect header.
 func WithProxyConnectHeader(header http.Header) Option {
 	return func(tr *transport) error {
-		if header != nil {
-			tr.ProxyConnectHeader = header
+		if header == nil {
+			return errors.NewErrInvalidOption("proxyConnectHeader", header)
 		}
+		tr.ProxyConnectHeader = header
 
 		return nil
 	}
 }
 
+//  WithMaxResponseHeaderBytes returns the option to set the max response header bytes.
 func WithMaxResponseHeaderBytes(bs int64) Option {
 	return func(tr *transport) error {
 		tr.MaxResponseHeaderBytes = bs
-
 		return nil
 	}
 }
 
+//  WithWriteBufferSize returns the option to set the write buffer size.
 func WithWriteBufferSize(bs int64) Option {
 	return func(tr *transport) error {
 		tr.WriteBufferSize = int(bs)
-
 		return nil
 	}
 }
 
+//  WithReadBufferSize returns the option to set the read buffer size.
 func WithReadBufferSize(bs int64) Option {
 	return func(tr *transport) error {
 		tr.ReadBufferSize = int(bs)
-
 		return nil
 	}
 }
 
+//  WithForceAttemptHTTP2 returns the option to force attempt HTTP2 for the HTTP transport.
 func WithForceAttemptHTTP2(force bool) Option {
 	return func(tr *transport) error {
 		tr.ForceAttemptHTTP2 = force
-
 		return nil
 	}
 }
 
+//  WithBackoffOpts returns the option to set the options to initialize backoff.
 func WithBackoffOpts(opts ...backoff.Option) Option {
 	return func(tr *transport) error {
+		if len(opts) == 0 {
+			return errors.NewErrInvalidOption("backoffOpts", opts)
+		}
 		if tr.backoffOpts == nil {
 			tr.backoffOpts = opts
+			return nil
 		}
 
 		tr.backoffOpts = append(tr.backoffOpts, opts...)
-
 		return nil
 	}
 }