x

internal/engine/experiment.go

@@ -46,6 +46,10 @@ func (emr *experimentMutableReport) Get() (report probeservices.ReportChannel) {
 	return
 }
 
+// TODO(bassosimone,DecFox): it would be nice if `*experiment` depended
+// on an interface rather than depending on the concrete session, because
+// that will allow us to write tests using mocks much more easily.
+
 // experiment implements [model.Experiment].
 type experiment struct {
 	byteCounter   *bytecounter.Counter

internal/engine/experiment_test.go

@@ -1,9 +1,14 @@
 package engine
 
 import (
+	"sync"
 	"testing"
+	"time"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/enginelocate"
+	"github.com/ooni/probe-cli/v3/internal/experiment/dnscheck"
 	"github.com/ooni/probe-cli/v3/internal/experiment/example"
 	"github.com/ooni/probe-cli/v3/internal/experiment/signal"
 	"github.com/ooni/probe-cli/v3/internal/model"
@@ -105,3 +110,84 @@ func TestExperimentMeasurementSummaryKeys(t *testing.T) {
 		}
 	})
 }
+
+// This test ensures that (*experiment).newMeasurement is working as intended.
+func TestExperimentNewMeasurement(t *testing.T) {
+	// create a session for testing that does not use the network at all
+	sess := newSessionForTestingNoLookups(t)
+
+	// create a conventional time for starting the experiment
+	t0 := time.Date(2024, 6, 27, 10, 33, 0, 0, time.UTC)
+
+	// create the experiment
+	exp := &experiment{
+		byteCounter: bytecounter.New(),
+		callbacks:   model.NewPrinterCallbacks(model.DiscardLogger),
+		measurer:    &dnscheck.Measurer{},
+		mrep: &experimentMutableReport{
+			mu:     sync.Mutex{},
+			report: nil,
+		},
+		session:       sess,
+		testName:      "dnscheck",
+		testStartTime: t0.Format(model.MeasurementDateFormat),
+		testVersion:   "0.1.0",
+	}
+
+	// create the richer input target
+	target := &dnscheck.Target{
+		Config: &dnscheck.Config{
+			DefaultAddrs: "8.8.8.8 2001:4860:4860::8888",
+			HTTP3Enabled: true,
+		},
+		URL: "https://dns.google/dns-query",
+	}
+
+	// create measurement
+	meas := exp.newMeasurement(target)
+
+	// make sure the input is correctly serialized
+	t.Run("Input", func(t *testing.T) {
+		if meas.Input != "https://dns.google/dns-query" {
+			t.Fatal("unexpected meas.Input")
+		}
+	})
+
+	// make sure the options are correctly serialized
+	t.Run("Options", func(t *testing.T) {
+		expectOptions := []string{`DefaultAddrs=8.8.8.8 2001:4860:4860::8888`, `HTTP3Enabled=true`}
+		if diff := cmp.Diff(expectOptions, meas.Options); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+
+	// make sure we've got the expected annotation keys
+	t.Run("Annotations", func(t *testing.T) {
+		const (
+			expected = 1 << iota
+			got
+		)
+		m := map[string]int{
+			"architecture":   expected,
+			"engine_name":    expected,
+			"engine_version": expected,
+			"go_version":     expected,
+			"platform":       expected,
+			"vcs_modified":   expected,
+			"vcs_revision":   expected,
+			"vcs_time":       expected,
+			"vcs_tool":       expected,
+		}
+		for key := range meas.Annotations {
+			m[key] |= got
+		}
+		for key, value := range m {
+			if value != expected|got {
+				t.Fatal("expected", expected|got, "for", key, "got", value)
+			}
+		}
+	})
+
+	// TODO(bassosimone,DecFox): this is the correct place where to
+	// add more tests regarding how we create measurements.
+}

internal/experimentconfig/experimentconfig.go

@@ -16,7 +16,9 @@ import (
 //
 // 1. we do not serialize options whose name starts with "Safe";
 //
-// 2. we only serialize scalar values.
+// 2. we only serialize scalar values;
+//
+// 3. we never serialize any zero values.
 //
 // This method MUST be passed a pointer to a struct. Otherwise, the return
 // value will be a zero-length list (either nil or empty).
@@ -49,7 +51,7 @@ func DefaultOptionsSerializer(config any) (options []string) {
 			continue
 		}
 
-		// add the field iff it's a scalar
+		// add the field iff it's a nonzero scalar
 		switch fieldval.Kind() {
 		case reflect.Bool,
 			reflect.Int,
@@ -65,7 +67,10 @@ func DefaultOptionsSerializer(config any) (options []string) {
 			reflect.Float32,
 			reflect.Float64,
 			reflect.String:
-			options = append(options, fmt.Sprintf("%s=%s", fieldtype.Name, fieldval.Interface()))
+			if fieldval.IsZero() {
+				continue
+			}
+			options = append(options, fmt.Sprintf("%s=%v", fieldtype.Name, fieldval.Interface()))
 
 		default:
 			// nothing

internal/model/experiment.go

@@ -104,9 +104,8 @@ type ExperimentTarget interface {
 	// using the command line `-O option=value` syntax.
 	//
 	// This method MUST NOT serialize all the options whose name
-	// starts with the "Safe" prefix. This method MAY skip serializing
-	// sensitive options and options we cannot serialize into a list
-	// of strings (e.g., objects and lists).
+	// starts with the "Safe" prefix. This method MUST skip serializing
+	// sensitive options, non-scalar options, and zero value options.
 	//
 	// Consider using the [experimentconfig] package to serialize.
 	Options() []string