feat: add echcheck to the experimental suite (#1217)

## Checklist

- [x] I have read the [contribution
guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request:
ooni/probe#1453
ooni/probe#2547
- [x] if you changed anything related to how experiments work and you
need to reflect these changes in the ooni/spec repository, please link
to the related ooni/spec pull request:
https://github.com/kelmenhorst/spec/tree/echcheck-spec
- [x] if you changed code inside an experiment, make sure you bump its
version number

## Description

This diff does two things:

1. Add `echcheck` to the experimental nettest suite. We do not provide
input which causes the experiment to use the default URL
`https://crypto.cloudflare.com/cdn-cgi/trace` (URL
[proposed](ooni/probe#1453 (comment))
by @eighthave). With this single input configuration we can collect the
first experimental ECH measurements.

2. Add netem tests to `echcheck`, while keeping a "real-internet" test
to connect to an ECH-enabled server.

---------

Co-authored-by: Simone Basso <[email protected]>

cmd/ooniprobe/internal/nettests/echcheck.go

@@ -0,0 +1,14 @@
+package nettests
+
+// ECHCheck nettest implementation.
+type ECHCheck struct{}
+
+// Run starts the nettest.
+func (n ECHCheck) Run(ctl *Controller) error {
+	builder, err := ctl.Session.NewExperimentBuilder("echcheck")
+	if err != nil {
+		return err
+	}
+	// providing empty input causes the experiment to use the default URL
+	return ctl.Run(builder, []string{})
+}

cmd/ooniprobe/internal/nettests/groups.go

@@ -55,6 +55,7 @@ var All = map[string]Group{
 		Label: "Experimental Nettests",
 		Nettests: []Nettest{
 			DNSCheck{},
+			ECHCheck{},
 			STUNReachability{},
 			TorSf{},
 			VanillaTor{},

internal/experiment/echcheck/handshake_test.go

@@ -31,7 +31,7 @@ func TestHandshake(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	result := handshakeWithEch(ctx, conn, time.Now(), parsed.Host, "example.org")
+	result := handshakeWithEch(ctx, conn, time.Now(), parsed.Host, "crypto.cloudflare.com")
 	if result == nil {
 		t.Fatal("expected result")
 	}

internal/experiment/echcheck/measure.go

@@ -14,9 +14,9 @@ import (
 )
 
 const (
-	testName      = "echcheck"
-	testVersion   = "0.1.1"
-	defaultDomain = "https://example.org"
+	testName    = "echcheck"
+	testVersion = "0.1.2"
+	defaultURL  = "https://crypto.cloudflare.com/cdn-cgi/trace"
 )
 
 var (
@@ -54,7 +54,7 @@ func (m *Measurer) Run(
 	args *model.ExperimentArgs,
 ) error {
 	if args.Measurement.Input == "" {
-		args.Measurement.Input = defaultDomain
+		args.Measurement.Input = defaultURL
 	}
 	parsed, err := url.Parse(string(args.Measurement.Input))
 	if err != nil {

internal/experiment/echcheck/measure_test.go

@@ -4,94 +4,127 @@ import (
 	"context"
 	"testing"
 
-	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/legacy/mockable"
+	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netemx"
 )
 
 func TestNewExperimentMeasurer(t *testing.T) {
 	measurer := NewExperimentMeasurer(Config{})
 	if measurer.ExperimentName() != "echcheck" {
 		t.Fatal("unexpected name")
 	}
-	if measurer.ExperimentVersion() != "0.1.1" {
+	if measurer.ExperimentVersion() != "0.1.2" {
 		t.Fatal("unexpected version")
 	}
 }
 
-func TestMeasurerMeasureWithInvalidInput(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel() // immediately cancel the context
-	sess := &mockable.Session{MockableLogger: log.Log}
-	callbacks := model.NewPrinterCallbacks(sess.Logger())
-	measurer := NewExperimentMeasurer(Config{})
-	measurement := &model.Measurement{
-		Input: "http://example.org",
-	}
-	args := &model.ExperimentArgs{
-		Callbacks:   callbacks,
-		Measurement: measurement,
-		Session:     sess,
-	}
-	err := measurer.Run(
-		ctx,
-		args,
-	)
-	if err == nil {
-		t.Fatal("expected an error here")
+// qaenv creates a [netemx.QAEnv] with a single crypto.cloudflare.com test server and a DoH server.
+func qaenv() *netemx.QAEnv {
+	cfg := []*netemx.ScenarioDomainAddresses{
+		{
+			Domains:          []string{"crypto.cloudflare.com"},
+			Addresses:        []string{"130.192.91.7"},
+			Role:             netemx.ScenarioRoleWebServer,
+			WebServerFactory: netemx.ExampleWebPageHandlerFactory(),
+		},
+		{
+			Domains:   []string{"mozilla.cloudflare-dns.com"},
+			Addresses: []string{"130.192.91.13"},
+			Role:      netemx.ScenarioRolePublicDNS,
+		},
 	}
+	return netemx.MustNewScenario(cfg)
 }
 
-func TestMeasurerMeasureWithInvalidInput2(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel() // immediately cancel the context
-	sess := &mockable.Session{MockableLogger: log.Log}
-	callbacks := model.NewPrinterCallbacks(sess.Logger())
+func TestMeasurerMeasureWithCancelledContext(t *testing.T) {
+	// create QAEnv
+	env := qaenv()
+	defer env.Close()
+
+	env.Do(func() {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel() // immediately cancel the context
+
+		// create measurer
+		measurer := NewExperimentMeasurer(Config{})
+		args := &model.ExperimentArgs{
+			Callbacks:   model.NewPrinterCallbacks(model.DiscardLogger),
+			Measurement: &model.Measurement{},
+			Session:     &mocks.Session{MockLogger: func() model.Logger { return model.DiscardLogger }},
+		}
+
+		// run measurement
+		err := measurer.Run(ctx, args)
+		if err == nil {
+			t.Fatal("expected an error here")
+		}
+		if err.Error() != "interrupted" {
+			t.Fatal("unexpected error type")
+		}
+	})
+
+}
+
+func TestMeasurerMeasureWithInvalidInput(t *testing.T) {
+	// create QAEnv
+	env := qaenv()
+	defer env.Close()
+
+	// create measurer
 	measurer := NewExperimentMeasurer(Config{})
-	measurement := &model.Measurement{
-		// leading space to test url.Parse failure
-		Input: " https://example.org",
-	}
 	args := &model.ExperimentArgs{
-		Callbacks:   callbacks,
-		Measurement: measurement,
-		Session:     sess,
+		Callbacks: model.NewPrinterCallbacks(model.DiscardLogger),
+		Measurement: &model.Measurement{
+			// leading space to test url.Parse failure
+			Input: " https://crypto.cloudflare.com/cdn-cgi/trace",
+		},
+		Session: &mocks.Session{MockLogger: func() model.Logger { return model.DiscardLogger }},
 	}
-	err := measurer.Run(
-		ctx,
-		args,
-	)
+	// run measurement
+	err := measurer.Run(context.Background(), args)
 	if err == nil {
 		t.Fatal("expected an error here")
 	}
+	if err.Error() != "input is not an URL" {
+		t.Fatal("unexpected error type")
+	}
 }
 
-func TestMeasurementSuccess(t *testing.T) {
+func TestMeasurementSuccessRealWorld(t *testing.T) {
 	if testing.Short() {
+		// this test uses the real internet so we want to skip this in short mode
 		t.Skip("skip test in short mode")
 	}
 
-	sess := &mockable.Session{MockableLogger: log.Log}
-	callbacks := model.NewPrinterCallbacks(sess.Logger())
+	// create measurer
 	measurer := NewExperimentMeasurer(Config{})
+	msrmnt := &model.Measurement{}
 	args := &model.ExperimentArgs{
-		Callbacks:   callbacks,
-		Measurement: &model.Measurement{},
-		Session:     sess,
+		Callbacks:   model.NewPrinterCallbacks(model.DiscardLogger),
+		Measurement: msrmnt,
+		Session:     &mocks.Session{MockLogger: func() model.Logger { return model.DiscardLogger }},
 	}
-	err := measurer.Run(
-		context.Background(),
-		args,
-	)
+
+	// run measurement
+	err := measurer.Run(context.Background(), args)
 	if err != nil {
 		t.Fatal("unexpected error: ", err)
 	}
 
+	// check results
 	summary, err := measurer.GetSummaryKeys(&model.Measurement{})
 	if err != nil {
-		t.Fatal(err)
+		t.Fatal("unexpected error: ", err)
 	}
 	if summary.(SummaryKeys).IsAnomaly != false {
 		t.Fatal("expected false")
 	}
+	tk := msrmnt.TestKeys.(TestKeys)
+	if tk.Control.Failure != nil {
+		t.Fatal("unexpected control failure:", *tk.Control.Failure)
+	}
+	if tk.Target.Failure != nil {
+		t.Fatal("unexpected target failure:", *tk.Target.Failure)
+	}
 }