chore: improve testing and increase coverage (#794)

This diff improves testing and increases coverage inside the
./internal/netxlite and ./internal/tracex packages.

See https://github.com/ooni/probe/issues/2121

internal/netxlite/getaddrinfo_cgo.go

@@ -207,23 +207,6 @@ func (state *getaddrinfoState) addrinfoToString(r *C.struct_addrinfo) (string, e
 	}
 }
 
-// staticAddrinfoWithInvalidFamily is an helper to construct an addrinfo struct
-// that we use in testing. (We cannot call CGO directly from tests.)
-func staticAddrinfoWithInvalidFamily() *C.struct_addrinfo {
-	var value C.struct_addrinfo       // zeroed by Go
-	value.ai_socktype = C.SOCK_STREAM // this is what the code expects
-	value.ai_family = 0               // but 0 is not AF_INET{,6}
-	return &value
-}
-
-// staticAddrinfoWithInvalidSocketType is an helper to construct an addrinfo struct
-// that we use in testing. (We cannot call CGO directly from tests.)
-func staticAddrinfoWithInvalidSocketType() *C.struct_addrinfo {
-	var value C.struct_addrinfo      // zeroed by Go
-	value.ai_socktype = C.SOCK_DGRAM // not SOCK_STREAM
-	return &value
-}
-
 // getaddrinfoCopyIP copies a net.IP.
 //
 // This function is adapted from copyIP

internal/netxlite/http.go

@@ -177,29 +177,29 @@ func NewOOHTTPBaseTransport(dialer model.Dialer, tlsDialer model.TLSDialer) mode
 
 	// Ensure we correctly forward CloseIdleConnections.
 	return &httpTransportConnectionsCloser{
-		HTTPTransport: &stdlibTransport{&oohttp.StdlibTransport{Transport: txp}},
+		HTTPTransport: &httpTransportStdlib{&oohttp.StdlibTransport{Transport: txp}},
 		Dialer:        dialer,
 		TLSDialer:     tlsDialer,
 	}
 }
 
 // stdlibTransport wraps oohttp.StdlibTransport to add .Network()
-type stdlibTransport struct {
+type httpTransportStdlib struct {
 	StdlibTransport *oohttp.StdlibTransport
 }
 
-var _ model.HTTPTransport = &stdlibTransport{}
+var _ model.HTTPTransport = &httpTransportStdlib{}
 
-func (txp *stdlibTransport) CloseIdleConnections() {
+func (txp *httpTransportStdlib) CloseIdleConnections() {
 	txp.StdlibTransport.CloseIdleConnections()
 }
 
-func (txp *stdlibTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+func (txp *httpTransportStdlib) RoundTrip(req *http.Request) (*http.Response, error) {
 	return txp.StdlibTransport.RoundTrip(req)
 }
 
 // Network implements HTTPTransport.Network.
-func (txp *stdlibTransport) Network() string {
+func (txp *httpTransportStdlib) Network() string {
 	return "tcp"
 }
 

internal/netxlite/http_test.go

@@ -272,7 +272,7 @@ func TestNewHTTPTransport(t *testing.T) {
 		if tlsWithReadTimeout.TLSDialer != td {
 			t.Fatal("invalid tls dialer")
 		}
-		stdlib := connectionsCloser.HTTPTransport.(*stdlibTransport)
+		stdlib := connectionsCloser.HTTPTransport.(*httpTransportStdlib)
 		if !stdlib.StdlibTransport.ForceAttemptHTTP2 {
 			t.Fatal("invalid ForceAttemptHTTP2")
 		}
@@ -292,6 +292,7 @@ func TestNewHTTPTransport(t *testing.T) {
 }
 
 func TestHTTPDialerWithReadTimeout(t *testing.T) {
+	t.Run("DialContext", func(t *testing.T) {
 		t.Run("on success", func(t *testing.T) {
 			var (
 				calledWithZeroTime    bool
@@ -359,9 +360,11 @@ func TestHTTPDialerWithReadTimeout(t *testing.T) {
 				t.Fatal("expected nil conn here")
 			}
 		})
+	})
 }
 
 func TestHTTPTLSDialerWithReadTimeout(t *testing.T) {
+	t.Run("DialContext", func(t *testing.T) {
 		t.Run("on success", func(t *testing.T) {
 			var (
 				calledWithZeroTime    bool
@@ -457,6 +460,7 @@ func TestHTTPTLSDialerWithReadTimeout(t *testing.T) {
 				t.Fatal("not called")
 			}
 		})
+	})
 }
 
 func TestNewHTTPTransportStdlib(t *testing.T) {

internal/netxlite/quic.go

@@ -140,7 +140,7 @@ func (d *quicDialerQUICGo) DialContext(ctx context.Context, network string,
 		pconn.Close() // we own it on failure
 		return nil, err
 	}
-	return &quicConnectionOwnsConn{EarlyConnection: qconn, conn: pconn}, nil
+	return newQUICConnectionOwnsConn(qconn, pconn), nil
 }
 
 func (d *quicDialerQUICGo) dialEarlyContext(ctx context.Context,
@@ -183,6 +183,8 @@ type quicDialerHandshakeCompleter struct {
 	Dialer model.QUICDialer
 }
 
+var _ model.QUICDialer = &quicDialerHandshakeCompleter{}
+
 // DialContext implements model.QUICDialer.DialContext.
 func (d *quicDialerHandshakeCompleter) DialContext(
 	ctx context.Context, network, address string,
@@ -214,6 +216,10 @@ type quicConnectionOwnsConn struct {
 	conn model.UDPLikeConn
 }
 
+func newQUICConnectionOwnsConn(qconn quic.EarlyConnection, pconn model.UDPLikeConn) *quicConnectionOwnsConn {
+	return &quicConnectionOwnsConn{EarlyConnection: qconn, conn: pconn}
+}
+
 // CloseWithError implements quic.EarlyConnection.CloseWithError.
 func (qconn *quicConnectionOwnsConn) CloseWithError(
 	code quic.ApplicationErrorCode, reason string) error {

internal/netxlite/quic_test.go

@@ -302,6 +302,31 @@ func TestQUICDialerQUICGo(t *testing.T) {
 				t.Fatal("the ServerName field must match")
 			}
 		})
+
+		t.Run("returns a quicDialerOwnConn in case of success", func(t *testing.T) {
+			tlsConfig := &tls.Config{
+				ServerName: "dns.google",
+			}
+			fakeconn := &mocks.QUICEarlyConnection{}
+			systemdialer := quicDialerQUICGo{
+				QUICListener: &quicListenerStdlib{},
+				mockDialEarlyContext: func(ctx context.Context, pconn net.PacketConn,
+					remoteAddr net.Addr, host string, tlsConfig *tls.Config,
+					quicConfig *quic.Config) (quic.EarlyConnection, error) {
+					return fakeconn, nil
+				},
+			}
+			ctx := context.Background()
+			qconn, err := systemdialer.DialContext(
+				ctx, "udp", "8.8.8.8:443", tlsConfig, &quic.Config{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			connOwner := qconn.(*quicConnectionOwnsConn)
+			if connOwner.EarlyConnection != fakeconn {
+				t.Fatal("invalid underlying conn")
+			}
+		})
 	})
 }
 
@@ -406,6 +431,33 @@ func TestQUICDialerHandshakeCompleter(t *testing.T) {
 	})
 }
 
+func TestQUICConnectionOwnsConn(t *testing.T) {
+	var (
+		quicClose bool
+		udpClose  bool
+	)
+	qconn := &mocks.QUICEarlyConnection{
+		MockCloseWithError: func(code quic.ApplicationErrorCode, reason string) error {
+			quicClose = true
+			return nil
+		},
+	}
+	pconn := &mocks.UDPLikeConn{
+		MockClose: func() error {
+			udpClose = true
+			return nil
+		},
+	}
+	conn := newQUICConnectionOwnsConn(qconn, pconn)
+	conn.CloseWithError(0, "")
+	if !quicClose {
+		t.Fatal("did not call qconn.CloseWithError")
+	}
+	if !udpClose {
+		t.Fatal("did not call pconn.Close")
+	}
+}
+
 func TestQUICDialerResolver(t *testing.T) {
 	t.Run("CloseIdleConnections", func(t *testing.T) {
 		var (
@@ -518,6 +570,27 @@ func TestQUICDialerResolver(t *testing.T) {
 				t.Fatal("gotTLSConfig.ServerName has not been set")
 			}
 		})
+
+		t.Run("on success", func(t *testing.T) {
+			expectedQConn := &mocks.QUICEarlyConnection{}
+			dialer := &quicDialerResolver{
+				Resolver: NewResolverStdlib(log.Log),
+				Dialer: &mocks.QUICDialer{
+					MockDialContext: func(ctx context.Context, network, address string,
+						tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) {
+						return expectedQConn, nil
+					},
+				}}
+			qconn, err := dialer.DialContext(
+				context.Background(), "udp", "8.8.4.4:443",
+				&tls.Config{}, &quic.Config{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if qconn != expectedQConn {
+				t.Fatal("unexpected underlying qconn")
+			}
+		})
 	})
 
 	t.Run("lookup host with address", func(t *testing.T) {

internal/netxlite/resolverparallel.go

@@ -22,6 +22,8 @@ type ParallelResolver struct {
 	Txp model.DNSTransport
 }
 
+var _ model.Resolver = &ParallelResolver{}
+
 // UnwrappedParallelResolver creates a new ParallelResolver instance. This instance is
 // not wrapped and you should wrap if before using it.
 func NewUnwrappedParallelResolver(t model.DNSTransport) *ParallelResolver {

internal/netxlite/resolverserial.go

@@ -33,6 +33,8 @@ type SerialResolver struct {
 	Txp model.DNSTransport
 }
 
+var _ model.Resolver = &SerialResolver{}
+
 // NewUnwrappedSerialResolver creates a new, and unwrapped, SerialResolver instance.
 func NewUnwrappedSerialResolver(t model.DNSTransport) *SerialResolver {
 	return &SerialResolver{

internal/tracex/event_test.go

@@ -0,0 +1,36 @@
+package tracex
+
+import "testing"
+
+func TestUnusedEventsNames(t *testing.T) {
+	// Tests that we don't break the names of events we're currently
+	// not getting the name of directly even if they're saved.
+
+	t.Run("EventQUICHandshakeStart", func(t *testing.T) {
+		ev := &EventQUICHandshakeStart{}
+		if ev.Name() != "quic_handshake_start" {
+			t.Fatal("invalid event name")
+		}
+	})
+
+	t.Run("EventQUICHandshakeDone", func(t *testing.T) {
+		ev := &EventQUICHandshakeDone{}
+		if ev.Name() != "quic_handshake_done" {
+			t.Fatal("invalid event name")
+		}
+	})
+
+	t.Run("EventTLSHandshakeStart", func(t *testing.T) {
+		ev := &EventTLSHandshakeStart{}
+		if ev.Name() != "tls_handshake_start" {
+			t.Fatal("invalid event name")
+		}
+	})
+
+	t.Run("EventTLSHandshakeDone", func(t *testing.T) {
+		ev := &EventTLSHandshakeDone{}
+		if ev.Name() != "tls_handshake_done" {
+			t.Fatal("invalid event name")
+		}
+	})
+}

internal/tracex/quic_test.go

@@ -177,6 +177,14 @@ func TestQUICDialerSaver(t *testing.T) {
 	})
 }
 
+func TestWrapQUICListener(t *testing.T) {
+	var saver *Saver
+	ql := &mocks.QUICListener{}
+	if saver.WrapQUICListener(ql) != ql {
+		t.Fatal("unexpected result")
+	}
+}
+
 func TestQUICListenerSaver(t *testing.T) {
 	t.Run("on failure", func(t *testing.T) {
 		expected := errors.New("mocked error")

internal/tracex/resolver_test.go

@@ -15,7 +15,16 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
+func TestWrapResolver(t *testing.T) {
+	var saver *Saver
+	reso := &mocks.Resolver{}
+	if saver.WrapResolver(reso) != reso {
+		t.Fatal("unexpected result")
+	}
+}
+
 func TestResolverSaver(t *testing.T) {
+	t.Run("LookupHost", func(t *testing.T) {
 		t.Run("on failure", func(t *testing.T) {
 			expected := netxlite.ErrOODNSNoSuchHost
 			saver := &Saver{}
@@ -103,9 +112,96 @@ func TestResolverSaver(t *testing.T) {
 				t.Fatal("the saved time is wrong")
 			}
 		})
+	})
+
+	t.Run("Network", func(t *testing.T) {
+		saver := &Saver{}
+		child := &mocks.Resolver{
+			MockNetwork: func() string {
+				return "x"
+			},
+		}
+		reso := saver.WrapResolver(child)
+		if reso.Network() != "x" {
+			t.Fatal("unexpected result")
+		}
+	})
+
+	t.Run("Address", func(t *testing.T) {
+		saver := &Saver{}
+		child := &mocks.Resolver{
+			MockAddress: func() string {
+				return "x"
+			},
+		}
+		reso := saver.WrapResolver(child)
+		if reso.Address() != "x" {
+			t.Fatal("unexpected result")
+		}
+	})
+
+	t.Run("LookupHTTPS", func(t *testing.T) {
+		expected := errors.New("mocked")
+		saver := &Saver{}
+		child := &mocks.Resolver{
+			MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) {
+				return nil, expected
+			},
+		}
+		reso := saver.WrapResolver(child)
+		https, err := reso.LookupHTTPS(context.Background(), "dns.google")
+		if !errors.Is(err, expected) {
+			t.Fatal("unexpected err", err)
+		}
+		if https != nil {
+			t.Fatal("expected nil")
+		}
+	})
+
+	t.Run("LookupNS", func(t *testing.T) {
+		expected := errors.New("mocked")
+		saver := &Saver{}
+		child := &mocks.Resolver{
+			MockLookupNS: func(ctx context.Context, domain string) ([]*net.NS, error) {
+				return nil, expected
+			},
+		}
+		reso := saver.WrapResolver(child)
+		ns, err := reso.LookupNS(context.Background(), "dns.google")
+		if !errors.Is(err, expected) {
+			t.Fatal("unexpected err", err)
+		}
+		if len(ns) != 0 {
+			t.Fatal("expected zero length array")
+		}
+	})
+
+	t.Run("CloseIdleConnections", func(t *testing.T) {
+		var called bool
+		saver := &Saver{}
+		child := &mocks.Resolver{
+			MockCloseIdleConnections: func() {
+				called = true
+			},
+		}
+		reso := saver.WrapResolver(child)
+		reso.CloseIdleConnections()
+		if !called {
+			t.Fatal("not called")
+		}
+	})
+}
+
+func TestWrapDNSTransport(t *testing.T) {
+	var saver *Saver
+	txp := &mocks.DNSTransport{}
+	if saver.WrapDNSTransport(txp) != txp {
+		t.Fatal("unexpected result")
+	}
 }
 
 func TestDNSTransportSaver(t *testing.T) {
+	t.Run("RoundTrip", func(t *testing.T) {
 		t.Run("on failure", func(t *testing.T) {
 			expected := netxlite.ErrOODNSNoSuchHost
 			saver := &Saver{}
@@ -230,6 +326,61 @@ func TestDNSTransportSaver(t *testing.T) {
 				t.Fatal("the saved time is wrong")
 			}
 		})
+	})
+
+	t.Run("Network", func(t *testing.T) {
+		saver := &Saver{}
+		child := &mocks.DNSTransport{
+			MockNetwork: func() string {
+				return "x"
+			},
+		}
+		txp := saver.WrapDNSTransport(child)
+		if txp.Network() != "x" {
+			t.Fatal("unexpected result")
+		}
+	})
+
+	t.Run("Address", func(t *testing.T) {
+		saver := &Saver{}
+		child := &mocks.DNSTransport{
+			MockAddress: func() string {
+				return "x"
+			},
+		}
+		txp := saver.WrapDNSTransport(child)
+		if txp.Address() != "x" {
+			t.Fatal("unexpected result")
+		}
+	})
+
+	t.Run("CloseIdleConnections", func(t *testing.T) {
+		var called bool
+		saver := &Saver{}
+		child := &mocks.DNSTransport{
+			MockCloseIdleConnections: func() {
+				called = true
+			},
+		}
+		txp := saver.WrapDNSTransport(child)
+		txp.CloseIdleConnections()
+		if !called {
+			t.Fatal("not called")
+		}
+	})
+
+	t.Run("RequiresPadding", func(t *testing.T) {
+		saver := &Saver{}
+		child := &mocks.DNSTransport{
+			MockRequiresPadding: func() bool {
+				return true
+			},
+		}
+		txp := saver.WrapDNSTransport(child)
+		if !txp.RequiresPadding() {
+			t.Fatal("unexpected result")
+		}
+	})
 }
 
 func newFakeResolverWithExplicitError(err error) model.Resolver {

internal/tracex/tls_test.go

@@ -12,6 +12,14 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model/mocks"
 )
 
+func TestWrapTLSHandshaker(t *testing.T) {
+	var saver *Saver
+	thx := &mocks.TLSHandshaker{}
+	if saver.WrapTLSHandshaker(thx) != thx {
+		t.Fatal("unexpected result")
+	}
+}
+
 func TestTLSHandshakerSaver(t *testing.T) {
 
 	t.Run("Handshake", func(t *testing.T) {