refactor(netxlite): improve tests for http and http3 (#487)

* refactor(netxlite): improve tests for http and http3

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

* Update internal/netxlite/http3.go

internal/netxlite/http3.go

@@ -3,6 +3,7 @@ package netxlite
 import (
 	"context"
 	"crypto/tls"
+	"io"
 	"net/http"
 
 	"github.com/lucas-clemente/quic-go"
@@ -13,19 +14,25 @@ import (
 // an http3.RoundTripper. This is necessary because the
 // http3.RoundTripper does not support DialContext.
 type http3Dialer struct {
-	Dialer QUICDialer
+	QUICDialer
 }
 
 // dial is like QUICContextDialer.DialContext but without context.
 func (d *http3Dialer) dial(network, address string, tlsConfig *tls.Config,
 	quicConfig *quic.Config) (quic.EarlySession, error) {
-	return d.Dialer.DialContext(
+	return d.QUICDialer.DialContext(
 		context.Background(), network, address, tlsConfig, quicConfig)
 }
 
+// http3RoundTripper is the abstract type of quic-go/http3.RoundTripper.
+type http3RoundTripper interface {
+	http.RoundTripper
+	io.Closer
+}
+
 // http3Transport is an HTTPTransport using the http3 protocol.
 type http3Transport struct {
-	child  *http3.RoundTripper
+	child  http3RoundTripper
 	dialer QUICDialer
 }
 

internal/netxlite/http3_test.go

@@ -1,42 +1,99 @@
 package netxlite
 
 import (
+	"context"
 	"crypto/tls"
+	"errors"
 	"net/http"
 	"testing"
 
-	"github.com/apex/log"
+	"github.com/lucas-clemente/quic-go"
+	"github.com/lucas-clemente/quic-go/http3"
 	"github.com/ooni/probe-cli/v3/internal/netxlite/mocks"
 )
 
-func TestHTTP3TransportWorks(t *testing.T) {
+func TestHTTP3Dialer(t *testing.T) {
-	d := &quicDialerResolver{
+	t.Run("Dial", func(t *testing.T) {
-		Dialer: &quicDialerQUICGo{
+		expected := errors.New("mocked error")
-			QUICListener: &quicListenerStdlib{},
+		d := &http3Dialer{
+			QUICDialer: &mocks.QUICDialer{
+				MockDialContext: func(ctx context.Context, network, address string, tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlySession, error) {
+					return nil, expected
+				},
 			},
-		Resolver: NewResolverSystem(log.Log),
 		}
-	txp := NewHTTP3Transport(d, &tls.Config{})
+		sess, err := d.dial("", "", &tls.Config{}, &quic.Config{})
-	client := &http.Client{Transport: txp}
+		if !errors.Is(err, expected) {
-	resp, err := client.Get("https://www.google.com/robots.txt")
+			t.Fatal("unexpected err", err)
-	if err != nil {
-		t.Fatal(err)
 		}
-	resp.Body.Close()
+		if sess != nil {
-	txp.CloseIdleConnections()
+			t.Fatal("unexpected resp")
+		}
+	})
 }
 
 func TestHTTP3TransportClosesIdleConnections(t *testing.T) {
-	var called bool
+	t.Run("CloseIdleConnections", func(t *testing.T) {
-	d := &mocks.QUICDialer{
+		var (
+			calledHTTP3  bool
+			calledDialer bool
+		)
+		txp := &http3Transport{
+			child: &mocks.HTTP3RoundTripper{
+				MockClose: func() error {
+					calledHTTP3 = true
+					return nil
+				},
+			},
+			dialer: &mocks.QUICDialer{
 				MockCloseIdleConnections: func() {
-			called = true
+					calledDialer = true
+				},
 			},
 		}
-	txp := NewHTTP3Transport(d, &tls.Config{})
+		txp.CloseIdleConnections()
-	client := &http.Client{Transport: txp}
+		if !calledHTTP3 || !calledDialer {
-	client.CloseIdleConnections()
-	if !called {
 			t.Fatal("not called")
 		}
+	})
+
+	t.Run("RoundTrip", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		txp := &http3Transport{
+			child: &mocks.HTTP3RoundTripper{
+				MockRoundTrip: func(req *http.Request) (*http.Response, error) {
+					return nil, expected
+				},
+			},
+		}
+		resp, err := txp.RoundTrip(&http.Request{})
+		if !errors.Is(err, expected) {
+			t.Fatal("unexpected err", err)
+		}
+		if resp != nil {
+			t.Fatal("unexpected resp")
+		}
+	})
+}
+
+func TestNewHTTP3Transport(t *testing.T) {
+	t.Run("creates the correct type chain", func(t *testing.T) {
+		qd := &mocks.QUICDialer{}
+		config := &tls.Config{}
+		txp := NewHTTP3Transport(qd, config)
+		h3txp := txp.(*http3Transport)
+		if h3txp.dialer != qd {
+			t.Fatal("invalid dialer")
+		}
+		h3 := h3txp.child.(*http3.RoundTripper)
+		if h3.Dial == nil {
+			t.Fatal("invalid Dial")
+		}
+		if !h3.DisableCompression {
+			t.Fatal("invalid DisableCompression")
+		}
+		if h3.TLSClientConfig != config {
+			t.Fatal("invalid TLSClientConfig")
+		}
+	})
 }

internal/netxlite/http_test.go

@@ -18,7 +18,9 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/netxlite/mocks"
 )
 
-func TestHTTPTransportLoggerFailure(t *testing.T) {
+func TestHTTPTransportLogger(t *testing.T) {
+	t.Run("RoundTrip", func(t *testing.T) {
+		t.Run("with failure", func(t *testing.T) {
 			txp := &httpTransportLogger{
 				Logger: log.Log,
 				HTTPTransport: &mocks.HTTPTransport{
@@ -35,9 +37,9 @@ func TestHTTPTransportLoggerFailure(t *testing.T) {
 			if resp != nil {
 				t.Fatal("expected nil response here")
 			}
-}
+		})
 
-func TestHTTPTransportLoggerFailureWithNoHostHeader(t *testing.T) {
+		t.Run("we add the host header", func(t *testing.T) {
 			foundHost := &atomicx.Int64{}
 			txp := &httpTransportLogger{
 				Logger: log.Log,
@@ -68,9 +70,9 @@ func TestHTTPTransportLoggerFailureWithNoHostHeader(t *testing.T) {
 			if foundHost.Load() != 1 {
 				t.Fatal("host header was not added")
 			}
-}
+		})
 
-func TestHTTPTransportLoggerSuccess(t *testing.T) {
+		t.Run("with success", func(t *testing.T) {
 			txp := &httpTransportLogger{
 				Logger: log.Log,
 				HTTPTransport: &mocks.HTTPTransport{
@@ -92,9 +94,10 @@ func TestHTTPTransportLoggerSuccess(t *testing.T) {
 			}
 			iox.ReadAllContext(context.Background(), resp.Body)
 			resp.Body.Close()
-}
+		})
+	})
 
-func TestHTTPTransportLoggerCloseIdleConnections(t *testing.T) {
+	t.Run("CloseIdleConnections", func(t *testing.T) {
 		calls := &atomicx.Int64{}
 		txp := &httpTransportLogger{
 			HTTPTransport: &mocks.HTTPTransport{
@@ -108,22 +111,61 @@ func TestHTTPTransportLoggerCloseIdleConnections(t *testing.T) {
 		if calls.Load() != 1 {
 			t.Fatal("not called")
 		}
+	})
 }
 
-func TestHTTPTransportWorks(t *testing.T) {
+func TestHTTPTransportConnectionsCloser(t *testing.T) {
-	d := NewDialerWithResolver(log.Log, NewResolverSystem(log.Log))
+	t.Run("CloseIdleConnections", func(t *testing.T) {
-	td := NewTLSDialer(d, NewTLSHandshakerStdlib(log.Log))
+		var (
-	txp := NewHTTPTransport(log.Log, d, td)
+			calledTxp    bool
+			calledDialer bool
+			calledTLS    bool
+		)
+		txp := &httpTransportConnectionsCloser{
+			HTTPTransport: &mocks.HTTPTransport{
+				MockCloseIdleConnections: func() {
+					calledTxp = true
+				},
+			},
+			Dialer: &mocks.Dialer{
+				MockCloseIdleConnections: func() {
+					calledDialer = true
+				},
+			},
+			TLSDialer: &mocks.TLSDialer{
+				MockCloseIdleConnections: func() {
+					calledTLS = true
+				},
+			},
+		}
+		txp.CloseIdleConnections()
+		if !calledDialer || !calledTLS || !calledTxp {
+			t.Fatal("not called")
+		}
+	})
+
+	t.Run("RoundTrip", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		txp := &httpTransportConnectionsCloser{
+			HTTPTransport: &mocks.HTTPTransport{
+				MockRoundTrip: func(req *http.Request) (*http.Response, error) {
+					return nil, expected
+				},
+			},
+		}
 		client := &http.Client{Transport: txp}
-	defer client.CloseIdleConnections()
+		resp, err := client.Get("https://www.google.com")
-	resp, err := client.Get("https://www.google.com/robots.txt")
+		if !errors.Is(err, expected) {
-	if err != nil {
+			t.Fatal("unexpected err", err)
-		t.Fatal(err)
 		}
-	resp.Body.Close()
+		if resp != nil {
+			t.Fatal("unexpected resp")
+		}
+	})
 }
 
-func TestHTTPTransportWithFailingDialer(t *testing.T) {
+func TestNewHTTPTransport(t *testing.T) {
+	t.Run("works as intended with failing dialer", func(t *testing.T) {
 		called := &atomicx.Int64{}
 		expected := errors.New("mocked error")
 		d := &dialerResolver{
@@ -152,59 +194,46 @@ func TestHTTPTransportWithFailingDialer(t *testing.T) {
 		if called.Load() < 1 {
 			t.Fatal("did not propagate CloseIdleConnections")
 		}
-}
+	})
 
-func TestNewHTTPTransport(t *testing.T) {
+	t.Run("creates the correct type chain", func(t *testing.T) {
 		d := &mocks.Dialer{}
 		td := &mocks.TLSDialer{}
 		txp := NewHTTPTransport(log.Log, d, td)
-	logtxp, okay := txp.(*httpTransportLogger)
+		logger := txp.(*httpTransportLogger)
-	if !okay {
+		if logger.Logger != log.Log {
-		t.Fatal("invalid type")
-	}
-	if logtxp.Logger != log.Log {
 			t.Fatal("invalid logger")
 		}
-	txpcc, okay := logtxp.HTTPTransport.(*httpTransportConnectionsCloser)
+		connectionsCloser := logger.HTTPTransport.(*httpTransportConnectionsCloser)
-	if !okay {
+		withReadTimeout := connectionsCloser.Dialer.(*httpDialerWithReadTimeout)
-		t.Fatal("invalid type")
+		if withReadTimeout.Dialer != d {
-	}
-	udt, okay := txpcc.Dialer.(*httpDialerWithReadTimeout)
-	if !okay {
-		t.Fatal("invalid type")
-	}
-	if udt.Dialer != d {
 			t.Fatal("invalid dialer")
 		}
-	utdt, okay := txpcc.TLSDialer.(*httpTLSDialerWithReadTimeout)
+		tlsWithReadTimeout := connectionsCloser.TLSDialer.(*httpTLSDialerWithReadTimeout)
-	if !okay {
+		if tlsWithReadTimeout.TLSDialer != td {
-		t.Fatal("invalid type")
-	}
-	if utdt.TLSDialer != td {
 			t.Fatal("invalid tls dialer")
 		}
-	stdwtxp, okay := txpcc.HTTPTransport.(*oohttp.StdlibTransport)
+		stdlib := connectionsCloser.HTTPTransport.(*oohttp.StdlibTransport)
-	if !okay {
+		if !stdlib.Transport.ForceAttemptHTTP2 {
-		t.Fatal("invalid type")
-	}
-	if !stdwtxp.Transport.ForceAttemptHTTP2 {
 			t.Fatal("invalid ForceAttemptHTTP2")
 		}
-	if !stdwtxp.Transport.DisableCompression {
+		if !stdlib.Transport.DisableCompression {
 			t.Fatal("invalid DisableCompression")
 		}
-	if stdwtxp.Transport.MaxConnsPerHost != 1 {
+		if stdlib.Transport.MaxConnsPerHost != 1 {
 			t.Fatal("invalid MaxConnPerHost")
 		}
-	if stdwtxp.Transport.DialTLSContext == nil {
+		if stdlib.Transport.DialTLSContext == nil {
 			t.Fatal("invalid DialTLSContext")
 		}
-	if stdwtxp.Transport.DialContext == nil {
+		if stdlib.Transport.DialContext == nil {
 			t.Fatal("invalid DialContext")
 		}
+	})
 }
 
 func TestHTTPDialerWithReadTimeout(t *testing.T) {
+	t.Run("on success", func(t *testing.T) {
 		var (
 			calledWithZeroTime    bool
 			calledWithNonZeroTime bool
@@ -252,9 +281,29 @@ func TestHTTPDialerWithReadTimeout(t *testing.T) {
 		if !calledWithZeroTime || !calledWithNonZeroTime {
 			t.Fatal("not called")
 		}
+	})
+
+	t.Run("on failure", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		d := &httpDialerWithReadTimeout{
+			Dialer: &mocks.Dialer{
+				MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
+					return nil, expected
+				},
+			},
+		}
+		conn, err := d.DialContext(context.Background(), "", "")
+		if !errors.Is(err, expected) {
+			t.Fatal("not the error we expected")
+		}
+		if conn != nil {
+			t.Fatal("expected nil conn here")
+		}
+	})
 }
 
 func TestHTTPTLSDialerWithReadTimeout(t *testing.T) {
+	t.Run("on success", func(t *testing.T) {
 		var (
 			calledWithZeroTime    bool
 			calledWithNonZeroTime bool
@@ -304,27 +353,9 @@ func TestHTTPTLSDialerWithReadTimeout(t *testing.T) {
 		if !calledWithZeroTime || !calledWithNonZeroTime {
 			t.Fatal("not called")
 		}
-}
+	})
 
-func TestHTTPDialerWithReadTimeoutDialingFailure(t *testing.T) {
+	t.Run("on failure", func(t *testing.T) {
-	expected := errors.New("mocked error")
-	d := &httpDialerWithReadTimeout{
-		Dialer: &mocks.Dialer{
-			MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
-				return nil, expected
-			},
-		},
-	}
-	conn, err := d.DialContext(context.Background(), "", "")
-	if !errors.Is(err, expected) {
-		t.Fatal("not the error we expected")
-	}
-	if conn != nil {
-		t.Fatal("expected nil conn here")
-	}
-}
-
-func TestHTTPTLSDialerWithReadTimeoutDialingFailure(t *testing.T) {
 		expected := errors.New("mocked error")
 		d := &httpTLSDialerWithReadTimeout{
 			TLSDialer: &mocks.TLSDialer{
@@ -340,9 +371,9 @@ func TestHTTPTLSDialerWithReadTimeoutDialingFailure(t *testing.T) {
 		if conn != nil {
 			t.Fatal("expected nil conn here")
 		}
-}
+	})
 
-func TestHTTPTLSDialerWithInvalidConnType(t *testing.T) {
+	t.Run("with invalid conn type", func(t *testing.T) {
 		var called bool
 		d := &httpTLSDialerWithReadTimeout{
 			TLSDialer: &mocks.TLSDialer{
@@ -366,4 +397,5 @@ func TestHTTPTLSDialerWithInvalidConnType(t *testing.T) {
 		if !called {
 			t.Fatal("not called")
 		}
+	})
 }

internal/netxlite/integration_test.go

@@ -0,0 +1,51 @@
+package netxlite_test
+
+import (
+	"crypto/tls"
+	"net/http"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+func TestHTTPTransport(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+
+	t.Run("works as intended", func(t *testing.T) {
+		d := netxlite.NewDialerWithResolver(log.Log, netxlite.NewResolverSystem(log.Log))
+		td := netxlite.NewTLSDialer(d, netxlite.NewTLSHandshakerStdlib(log.Log))
+		txp := netxlite.NewHTTPTransport(log.Log, d, td)
+		client := &http.Client{Transport: txp}
+		resp, err := client.Get("https://www.google.com/robots.txt")
+		if err != nil {
+			t.Fatal(err)
+		}
+		resp.Body.Close()
+		client.CloseIdleConnections()
+	})
+}
+
+func TestHTTP3Transport(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+
+	t.Run("works as intended", func(t *testing.T) {
+		d := netxlite.NewQUICDialerWithResolver(
+			netxlite.NewQUICListener(),
+			log.Log,
+			netxlite.NewResolverSystem(log.Log),
+		)
+		txp := netxlite.NewHTTP3Transport(d, &tls.Config{})
+		client := &http.Client{Transport: txp}
+		resp, err := client.Get("https://www.google.com/robots.txt")
+		if err != nil {
+			t.Fatal(err)
+		}
+		resp.Body.Close()
+		txp.CloseIdleConnections()
+	})
+}

internal/netxlite/mocks/http3.go

@@ -0,0 +1,19 @@
+package mocks
+
+import "net/http"
+
+// HTTP3RoundTripper allows mocking http3.RoundTripper.
+type HTTP3RoundTripper struct {
+	MockRoundTrip func(req *http.Request) (*http.Response, error)
+	MockClose     func() error
+}
+
+// RoundTrip calls MockRoundTrip.
+func (txp *HTTP3RoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	return txp.MockRoundTrip(req)
+}
+
+// Close calls MockClose.
+func (txp *HTTP3RoundTripper) Close() error {
+	return txp.MockClose()
+}

internal/netxlite/mocks/http3_test.go

@@ -0,0 +1,37 @@
+package mocks
+
+import (
+	"errors"
+	"net/http"
+	"testing"
+)
+
+func TestHTTP3RoundTripper(t *testing.T) {
+	t.Run("RoundTrip", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		txp := &HTTP3RoundTripper{
+			MockRoundTrip: func(req *http.Request) (*http.Response, error) {
+				return nil, expected
+			},
+		}
+		resp, err := txp.RoundTrip(&http.Request{})
+		if !errors.Is(err, expected) {
+			t.Fatal("unexpected err", err)
+		}
+		if resp != nil {
+			t.Fatal("unexpected resp")
+		}
+	})
+
+	t.Run("Close", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		txp := &HTTP3RoundTripper{
+			MockClose: func() error {
+				return expected
+			},
+		}
+		if err := txp.Close(); !errors.Is(err, expected) {
+			t.Fatal("unexpected err", err)
+		}
+	})
+}