diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index 200d50d..5a7a3d0 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -38,8 +38,7 @@ func (d *DialerResolver) DialContext(ctx context.Context, network, address strin if err != nil { return nil, err } - var addrs []string - addrs, err = d.lookupHost(ctx, onlyhost) + addrs, err := d.lookupHost(ctx, onlyhost) if err != nil { return nil, err } @@ -67,9 +66,12 @@ func (d *DialerResolver) lookupHost(ctx context.Context, hostname string) ([]str return d.Resolver.LookupHost(ctx, hostname) } -// DialerLogger is a Dialer with logging +// DialerLogger is a Dialer with logging. type DialerLogger struct { - Dialer + // Dialer is the underlying dialer. + Dialer Dialer + + // Logger is the underlying logger. Logger Logger } @@ -80,7 +82,11 @@ func (d *DialerLogger) DialContext(ctx context.Context, network, address string) d.Logger.Debugf("dial %s/%s...", address, network) start := time.Now() conn, err := d.Dialer.DialContext(ctx, network, address) - stop := time.Now() - d.Logger.Debugf("dial %s/%s... %+v in %s", address, network, err, stop.Sub(start)) - return conn, err + elapsed := time.Since(start) + if err != nil { + d.Logger.Debugf("dial %s/%s... %s in %s", address, network, err, elapsed) + return nil, err + } + d.Logger.Debugf("dial %s/%s... ok in %s", address, network, elapsed) + return conn, nil } diff --git a/internal/netxlite/dialer_test.go b/internal/netxlite/dialer_test.go index 6782ff0..cb1b8c6 100644 --- a/internal/netxlite/dialer_test.go +++ b/internal/netxlite/dialer_test.go @@ -14,10 +14,10 @@ import ( ) func TestDialerResolverNoPort(t *testing.T) { - dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: DefaultResolver} - conn, err := dialer.DialContext(context.Background(), "tcp", "antani.ooni.nu") - if err == nil { - t.Fatal("expected an error here") + dialer := &DialerResolver{Dialer: &net.Dialer{}, Resolver: DefaultResolver} + conn, err := dialer.DialContext(context.Background(), "tcp", "ooni.nu") + if err == nil || !strings.HasSuffix(err.Error(), "missing port in address") { + t.Fatal("not the error we expected", err) } if conn != nil { t.Fatal("expected a nil conn here") @@ -25,8 +25,10 @@ func TestDialerResolverNoPort(t *testing.T) { } func TestDialerResolverLookupHostAddress(t *testing.T) { - dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: MockableResolver{ - Err: errors.New("mocked error"), + dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return nil, errors.New("we should not call this function") + }, }} addrs, err := dialer.lookupHost(context.Background(), "1.1.1.1") if err != nil { @@ -39,35 +41,21 @@ func TestDialerResolverLookupHostAddress(t *testing.T) { func TestDialerResolverLookupHostFailure(t *testing.T) { expected := errors.New("mocked error") - dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: MockableResolver{ - Err: expected, + dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return nil, expected + }, }} - conn, err := dialer.DialContext(context.Background(), "tcp", "dns.google.com:853") + ctx := context.Background() + conn, err := dialer.DialContext(ctx, "tcp", "dns.google.com:853") if !errors.Is(err, expected) { - t.Fatal("not the error we expected") + t.Fatal("not the error we expected", err) } if conn != nil { t.Fatal("expected nil conn") } } -type MockableResolver struct { - Addresses []string - Err error -} - -func (r MockableResolver) LookupHost(ctx context.Context, host string) ([]string, error) { - return r.Addresses, r.Err -} - -func (r MockableResolver) Network() string { - return "mockable" -} - -func (r MockableResolver) Address() string { - return "" -} - func TestDialerResolverDialForSingleIPFails(t *testing.T) { dialer := &DialerResolver{Dialer: &netxmocks.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { @@ -89,8 +77,10 @@ func TestDialerResolverDialForManyIPFails(t *testing.T) { MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return nil, io.EOF }, - }, Resolver: MockableResolver{ - Addresses: []string{"1.1.1.1", "8.8.8.8"}, + }, Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return []string{"1.1.1.1", "8.8.8.8"}, nil + }, }} conn, err := dialer.DialContext(context.Background(), "tcp", "dot.dns:853") if !errors.Is(err, io.EOF) { @@ -110,8 +100,10 @@ func TestDialerResolverDialForManyIPSuccess(t *testing.T) { }, }, nil }, - }, Resolver: MockableResolver{ - Addresses: []string{"1.1.1.1", "8.8.8.8"}, + }, Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return []string{"1.1.1.1", "8.8.8.8"}, nil + }, }} conn, err := dialer.DialContext(context.Background(), "tcp", "dot.dns:853") if err != nil { @@ -123,6 +115,29 @@ func TestDialerResolverDialForManyIPSuccess(t *testing.T) { conn.Close() } +func TestDialerLoggerSuccess(t *testing.T) { + d := &DialerLogger{ + Dialer: &netxmocks.Dialer{ + MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + return &netxmocks.Conn{ + MockClose: func() error { + return nil + }, + }, nil + }, + }, + Logger: log.Log, + } + conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") + if err != nil { + t.Fatal(err) + } + if conn == nil { + t.Fatal("expected non-nil conn here") + } + conn.Close() +} + func TestDialerLoggerFailure(t *testing.T) { d := &DialerLogger{ Dialer: &netxmocks.Dialer{ @@ -141,18 +156,6 @@ func TestDialerLoggerFailure(t *testing.T) { } } -func TestDefaultDialerWorks(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() // fail immediately - conn, err := DefaultDialer.DialContext(ctx, "tcp", "8.8.8.8:853") - if err == nil || !strings.HasSuffix(err.Error(), "operation was canceled") { - t.Fatal("not the error we expected", err) - } - if conn != nil { - t.Fatal("expected nil conn here") - } -} - func TestDefaultDialerHasTimeout(t *testing.T) { expected := 15 * time.Second if DefaultDialer.Timeout != expected { diff --git a/internal/netxlite/legacy_test.go b/internal/netxlite/legacy_test.go index a6d7254..1b8fc21 100644 --- a/internal/netxlite/legacy_test.go +++ b/internal/netxlite/legacy_test.go @@ -14,7 +14,6 @@ func TestReduceErrors(t *testing.T) { t.Fatal("wrong result") } }) - t.Run("single error", func(t *testing.T) { err := errors.New("mocked error") result := ReduceErrors([]error{err}) @@ -22,7 +21,6 @@ func TestReduceErrors(t *testing.T) { t.Fatal("wrong result") } }) - t.Run("multiple errors", func(t *testing.T) { err1 := errors.New("mocked error #1") err2 := errors.New("mocked error #2") @@ -31,7 +29,6 @@ func TestReduceErrors(t *testing.T) { t.Fatal("wrong result") } }) - t.Run("multiple errors with meaningful ones", func(t *testing.T) { err1 := errors.New("mocked error #1") err2 := &errorx.ErrWrapper{ diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index 99d07e8..6f4b1f8 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -48,9 +48,13 @@ func (r ResolverLogger) LookupHost(ctx context.Context, hostname string) ([]stri r.Logger.Debugf("resolve %s...", hostname) start := time.Now() addrs, err := r.Resolver.LookupHost(ctx, hostname) - stop := time.Now() - r.Logger.Debugf("resolve %s... (%+v, %+v) in %s", hostname, addrs, err, stop.Sub(start)) - return addrs, err + elapsed := time.Since(start) + if err != nil { + r.Logger.Debugf("resolve %s... %s in %s", hostname, err, elapsed) + return nil, err + } + r.Logger.Debugf("resolve %s... %+v in %s", hostname, addrs, elapsed) + return addrs, nil } type resolverNetworker interface { diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go index 7490589..430bb25 100644 --- a/internal/netxlite/resolver_test.go +++ b/internal/netxlite/resolver_test.go @@ -2,13 +2,16 @@ package netxlite import ( "context" + "errors" "net" "testing" "github.com/apex/log" + "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/netxmocks" ) -func TestResolverSystemLookupHost(t *testing.T) { +func TestResolverSystemNetworkAddress(t *testing.T) { r := ResolverSystem{} if r.Network() != "system" { t.Fatal("invalid Network") @@ -16,6 +19,10 @@ func TestResolverSystemLookupHost(t *testing.T) { if r.Address() != "" { t.Fatal("invalid Address") } +} + +func TestResolverSystemWorksAsIntended(t *testing.T) { + r := ResolverSystem{} addrs, err := r.LookupHost(context.Background(), "dns.google.com") if err != nil { t.Fatal(err) @@ -25,27 +32,55 @@ func TestResolverSystemLookupHost(t *testing.T) { } } -func TestResolverLoggerWithFailure(t *testing.T) { +func TestResolverLoggerWithSuccess(t *testing.T) { + expected := []string{"1.1.1.1"} r := ResolverLogger{ - Logger: log.Log, - Resolver: DefaultResolver, + Logger: log.Log, + Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return expected, nil + }, + }, } - if r.Network() != "system" { - t.Fatal("invalid Network") + addrs, err := r.LookupHost(context.Background(), "dns.google") + if err != nil { + t.Fatal(err) } - if r.Address() != "" { - t.Fatal("invalid Address") + if diff := cmp.Diff(expected, addrs); diff != "" { + t.Fatal(diff) } - addrs, err := r.LookupHost(context.Background(), "nonexistent.antani") - if err == nil { - t.Fatal("expected an error here") +} + +func TestResolverLoggerWithFailure(t *testing.T) { + expected := errors.New("mocked error") + r := ResolverLogger{ + Logger: log.Log, + Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return nil, expected + }, + }, + } + addrs, err := r.LookupHost(context.Background(), "dns.google") + if !errors.Is(err, expected) { + t.Fatal("not the error we expected", err) } if addrs != nil { t.Fatal("expected nil addr here") } } -func TestResolverLoggerDefaultNetworkAddress(t *testing.T) { +func TestResolverLoggerChildNetworkAddress(t *testing.T) { + r := &ResolverLogger{Logger: log.Log, Resolver: DefaultResolver} + if r.Network() != "system" { + t.Fatal("invalid Network") + } + if r.Address() != "" { + t.Fatal("invalid Address") + } +} + +func TestResolverLoggerNoChildNetworkAddress(t *testing.T) { r := &ResolverLogger{Logger: log.Log, Resolver: &net.Resolver{}} if r.Network() != "logger" { t.Fatal("invalid Network") diff --git a/internal/netxmocks/conn.go b/internal/netxmocks/conn.go index 77826ad..a8c62af 100644 --- a/internal/netxmocks/conn.go +++ b/internal/netxmocks/conn.go @@ -17,42 +17,42 @@ type Conn struct { MockSetWriteDeadline func(t time.Time) error } -// Read implements net.Conn.Read +// Read calls MockRead. func (c *Conn) Read(b []byte) (int, error) { return c.MockRead(b) } -// Write implements net.Conn.Write +// Write calls MockWrite. func (c *Conn) Write(b []byte) (int, error) { return c.MockWrite(b) } -// Close implements net.Conn.Close +// Close calls MockClose. func (c *Conn) Close() error { return c.MockClose() } -// LocalAddr returns the local address +// LocalAddr class MockLocalAddr. func (c *Conn) LocalAddr() net.Addr { return c.MockLocalAddr() } -// RemoteAddr returns the remote address +// RemoteAddr calls MockRemoteAddr. func (c *Conn) RemoteAddr() net.Addr { return c.MockRemoteAddr() } -// SetDeadline sets the connection deadline. +// SetDeadline calls MockSetDeadline. func (c *Conn) SetDeadline(t time.Time) error { return c.MockSetDeadline(t) } -// SetReadDeadline sets the read deadline. +// SetReadDeadline calls MockSetReadDeadline. func (c *Conn) SetReadDeadline(t time.Time) error { return c.MockSetReadDeadline(t) } -// SetWriteDeadline sets the write deadline. +// SetWriteDeadline calls MockSetWriteDeadline. func (c *Conn) SetWriteDeadline(t time.Time) error { return c.MockSetWriteDeadline(t) } diff --git a/internal/netxmocks/dialer.go b/internal/netxmocks/dialer.go index c7d0dcb..f176a20 100644 --- a/internal/netxmocks/dialer.go +++ b/internal/netxmocks/dialer.go @@ -15,7 +15,7 @@ type Dialer struct { MockDialContext func(ctx context.Context, network, address string) (net.Conn, error) } -// DialContext implements Dialer.DialContext. +// DialContext calls MockDialContext. func (d *Dialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { return d.MockDialContext(ctx, network, address) } diff --git a/internal/netxmocks/resolver.go b/internal/netxmocks/resolver.go index 467aa62..89c996f 100644 --- a/internal/netxmocks/resolver.go +++ b/internal/netxmocks/resolver.go @@ -16,17 +16,17 @@ type Resolver struct { MockAddress func() string } -// LookupHost implements Resolver.LookupHost. +// LookupHost calls MockLookupHost. func (r *Resolver) LookupHost(ctx context.Context, domain string) ([]string, error) { return r.MockLookupHost(ctx, domain) } -// Address implements Resolver.Address. +// Address calls MockAddress. func (r *Resolver) Address() string { return r.MockAddress() } -// Network implements Resolver.Network. +// Network calls MockNetwork. func (r *Resolver) Network() string { return r.MockNetwork() }