refactor: only use shaping dialer for ndt7 and dash (#754)

See https://github.com/ooni/probe/issues/2112 for context.

While there, run `go fix -fix buildtag ./...`

internal/cmd/jafar/iptables/iptables_linux.go

@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package iptables
 

internal/cmd/jafar/iptables/iptables_unsupported.go

@@ -1,5 +1,4 @@
 //go:build !linux
-// +build !linux
 
 package iptables
 

internal/engine/experiment/dash/dash.go

@@ -259,8 +259,14 @@ func (m Measurer) Run(
 	httpClient := &http.Client{
 		Transport: netx.NewHTTPTransport(netx.Config{
 			ContextByteCounting: true,
-			DialSaver:           saver,
+			// Implements shaping if the user builds using `-tags shaping`
-			Logger:              sess.Logger(),
+			// See https://github.com/ooni/probe/issues/2112
+			Dialer: netxlite.NewMaybeShapingDialer(netx.NewDialer(netx.Config{
+				ContextByteCounting: true,
+				DialSaver:           saver,
+				Logger:              sess.Logger(),
+			})),
+			Logger: sess.Logger(),
 		}),
 	}
 	defer httpClient.CloseIdleConnections()

internal/engine/experiment/ndt7/dial.go

@@ -42,6 +42,9 @@ func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*
 		Logger:              mgr.logger,
 		ProxyURL:            mgr.proxyURL,
 	}, reso)
+	// Implements shaping if the user builds using `-tags shaping`
+	// See https://github.com/ooni/probe/issues/2112
+	dlr = netxlite.NewMaybeShapingDialer(dlr)
 	// We force using our bundled CA pool, which should fix
 	// https://github.com/ooni/probe/issues/2031
 	tlsConfig := &tls.Config{

internal/engine/netx/dialer/dialer.go

@@ -67,6 +67,5 @@ func New(config *Config, resolver model.Resolver) model.Dialer {
 	if config.ContextByteCounting {
 		d = &byteCounterDialer{Dialer: d}
 	}
-	d = &shapingDialer{Dialer: d}
 	return d
 }

internal/engine/netx/dialer/dialer_test.go

@@ -18,11 +18,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) {
 		ProxyURL:            &url.URL{},
 		ReadWriteSaver:      saver,
 	}, netxlite.DefaultResolver)
-	shd, ok := dlr.(*shapingDialer)
+	bcd, ok := dlr.(*byteCounterDialer)
-	if !ok {
-		t.Fatal("not a shapingDialer")
-	}
-	bcd, ok := shd.Dialer.(*byteCounterDialer)
 	if !ok {
 		t.Fatal("not a byteCounterDialer")
 	}

internal/engine/netx/dialer/shaping_disabled.go

@@ -1,24 +0,0 @@
-//go:build !shaping
-// +build !shaping
-
-package dialer
-
-import (
-	"context"
-	"net"
-
-	"github.com/ooni/probe-cli/v3/internal/model"
-)
-
-// shapingDialer ensures we don't use too much bandwidth
-// when using integration tests at GitHub. To select
-// the implementation with shaping use `-tags shaping`.
-type shapingDialer struct {
-	model.Dialer
-}
-
-// DialContext implements Dialer.DialContext
-func (d *shapingDialer) DialContext(
-	ctx context.Context, network, address string) (net.Conn, error) {
-	return d.Dialer.DialContext(ctx, network, address)
-}

internal/engine/netx/dialer/shaping_test.go

@@ -1,22 +0,0 @@
-package dialer
-
-import (
-	"net/http"
-	"testing"
-
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-func TestShapingDialerGood(t *testing.T) {
-	d := &shapingDialer{Dialer: netxlite.DefaultDialer}
-	txp := &http.Transport{DialContext: d.DialContext}
-	client := &http.Client{Transport: txp}
-	resp, err := client.Get("https://www.google.com/")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resp == nil {
-		t.Fatal("expected nil response here")
-	}
-	resp.Body.Close()
-}

internal/engine/session_nopsiphon.go

@@ -1,5 +1,4 @@
 //go:build !ooni_psiphon_config
-// +build !ooni_psiphon_config
 
 package engine
 

internal/engine/session_nopsiphon_test.go

@@ -1,5 +1,4 @@
 //go:build !ooni_psiphon_config
-// +build !ooni_psiphon_config
 
 package engine
 

internal/engine/session_psiphon.go

@@ -1,5 +1,4 @@
 //go:build ooni_psiphon_config
-// +build ooni_psiphon_config
 
 package engine
 

internal/engine/session_psiphon_test.go

@@ -1,5 +1,4 @@
 //go:build ooni_psiphon_config
-// +build ooni_psiphon_config
 
 package engine
 

internal/netxlite/shaping.go

@@ -0,0 +1,15 @@
+package netxlite
+
+import "github.com/ooni/probe-cli/v3/internal/model"
+
+// NewMaybeShapingDialer takes in input a model.Dialer and returns in output another
+// model.Dialer that MAY dial connections with I/O shaping, depending on whether
+// the user builds with or without the `-tags shaping` CLI flag.
+//
+// We typically use `-tags shaping` when running integration tests for dash and ndt7 to
+// avoiod hammering m-lab servers from the very-fast GitHub CI servers.
+//
+// See https://github.com/ooni/probe/issues/2112 for extra context.
+func NewMaybeShapingDialer(dialer model.Dialer) model.Dialer {
+	return newMaybeShapingDialer(dialer)
+}

internal/netxlite/shaping_otherwise.go

@@ -0,0 +1,11 @@
+//go:build !shaping
+
+package netxlite
+
+import (
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+func newMaybeShapingDialer(dialer model.Dialer) model.Dialer {
+	return dialer
+}

internal/netxlite/shaping_otherwise_test.go

@@ -0,0 +1,17 @@
+//go:build !shaping
+
+package netxlite
+
+import (
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+)
+
+func TestNewShapingDialer(t *testing.T) {
+	in := &mocks.Dialer{}
+	out := NewMaybeShapingDialer(in)
+	if in != out {
+		t.Fatal("expected to see the same pointer")
+	}
+}

internal/netxlite/shaping_shaping.go

@@ -1,7 +1,6 @@
 //go:build shaping
-// +build shaping
 
-package dialer
+package netxlite
 
 import (
 	"context"
@@ -11,9 +10,10 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
-// shapingDialer ensures we don't use too much bandwidth
+func newMaybeShapingDialer(dialer model.Dialer) model.Dialer {
-// when using integration tests at GitHub. To select
+	return &shapingDialer{dialer}
-// the implementation with shaping use `-tags shaping`.
+}
+
 type shapingDialer struct {
 	model.Dialer
 }

internal/netxlite/shaping_shaping_test.go

@@ -0,0 +1,66 @@
+//go:build shaping
+
+package netxlite
+
+import (
+	"context"
+	"errors"
+	"net"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+)
+
+func TestNewShapingDialerx(t *testing.T) {
+	t.Run("failure", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		d := &mocks.Dialer{
+			MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
+				return nil, expected
+			},
+		}
+		shd := NewMaybeShapingDialer(d)
+		conn, err := shd.DialContext(context.Background(), "tcp", "8.8.8.8:443")
+		if !errors.Is(err, expected) {
+			t.Fatal("unexpected err", err)
+		}
+		if conn != nil {
+			t.Fatal("expected nil conn")
+		}
+	})
+
+	t.Run("success", func(t *testing.T) {
+		expected := errors.New("mocked error")
+		uc := &mocks.Conn{
+			MockRead: func(b []byte) (int, error) {
+				return 0, expected
+			},
+			MockWrite: func(b []byte) (int, error) {
+				return 0, expected
+			},
+		}
+		d := &mocks.Dialer{
+			MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
+				return uc, nil
+			},
+		}
+		shd := NewMaybeShapingDialer(d)
+		conn, err := shd.DialContext(context.Background(), "tcp", "8.8.8.8:443")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, ok := conn.(*shapingConn); !ok {
+			t.Fatal("not shapingConn")
+		}
+		validateCountAndErr := func(count int, err error) {
+			if !errors.Is(err, expected) {
+				t.Fatal("unexpected err", err)
+			}
+			if count != 0 {
+				t.Fatal("expected zero")
+			}
+		}
+		validateCountAndErr(conn.Read(make([]byte, 16)))
+		validateCountAndErr(conn.Write(make([]byte, 16)))
+	})
+}