feat(webconnectivitylte): wire-in SummaryKeys (#1493)

Closes ooni/probe#2667.

While there, repair flaky unit test and explain why it was flaky.

internal/database/actions_test.go

@@ -450,6 +450,13 @@ func TestUpdateDatabaseMeasurementWithSummaryKeys(t *testing.T) {
 		ffiller := &testingx.FakeFiller{}
 		ffiller.Fill(sk)
 
+		// This field is not serialized, because historically it was used as an ABI to
+		// communicate between probe-engine and probe-cli, so we should not allow the fake
+		// filler to initialize it to a random value, otherwise we'll have issues in 50%
+		// of the cases where we would expect true and unmarshal false. For this reason,
+		// let's always set the field to the expected unmarshal value, i.e., false.
+		sk.IsAnomaly = false
+
 		if err := updateDatabaseMeasurementWithSummaryKeys(meas, sk); err != nil {
 			t.Fatal(err)
 		}

internal/experiment/webconnectivitylte/analysisclassic.go

@@ -148,17 +148,17 @@ func (tk *TestKeys) titleMatch() optional.Value[bool] {
 // setBlockingFalse implements analysisClassicTestKeysProxy.
 func (tk *TestKeys) setBlockingFalse() {
 	tk.Blocking = false
-	tk.Accessible = true
+	tk.Accessible = optional.Some(true)
 }
 
 // setBlockingNil implements analysisClassicTestKeysProxy.
 func (tk *TestKeys) setBlockingNil() {
 	if !tk.DNSConsistency.IsNone() && tk.DNSConsistency.Unwrap() == "inconsistent" {
 		tk.Blocking = "dns"
-		tk.Accessible = false
+		tk.Accessible = optional.Some(false)
 	} else {
 		tk.Blocking = nil
-		tk.Accessible = nil
+		tk.Accessible = optional.None[bool]()
 	}
 }
 
@@ -169,7 +169,7 @@ func (tk *TestKeys) setBlockingString(value string) {
 	} else {
 		tk.Blocking = value
 	}
-	tk.Accessible = false
+	tk.Accessible = optional.Some(false)
 }
 
 // setHTTPExperimentFailure implements analysisClassicTestKeysProxy.
@@ -181,10 +181,10 @@ func (tk *TestKeys) setHTTPExperimentFailure(value optional.Value[string]) {
 func (tk *TestKeys) setWebsiteDown() {
 	if !tk.DNSConsistency.IsNone() && tk.DNSConsistency.Unwrap() == "inconsistent" {
 		tk.Blocking = "dns"
-		tk.Accessible = false
+		tk.Accessible = optional.Some(false)
 	} else {
 		tk.Blocking = false
-		tk.Accessible = false
+		tk.Accessible = optional.Some(false)
 	}
 }
 

internal/experiment/webconnectivitylte/summary.go

@@ -0,0 +1,32 @@
+package webconnectivitylte
+
+import "github.com/ooni/probe-cli/v3/internal/model"
+
+var _ model.MeasurementSummaryKeysProvider = &TestKeys{}
+
+// SummaryKeys contains summary keys for this experiment.
+type SummaryKeys struct {
+	Accessible bool   `json:"accessible"`
+	Blocking   string `json:"blocking"`
+	IsAnomaly  bool   `json:"-"`
+}
+
+// MeasurementSummaryKeys implements model.MeasurementSummaryKeysProvider.
+func (tk *TestKeys) MeasurementSummaryKeys() model.MeasurementSummaryKeys {
+	// TODO(https://github.com/ooni/probe/issues/1684)
+	sk := &SummaryKeys{}
+	switch v := tk.Blocking.(type) {
+	case string:
+		sk.IsAnomaly = true
+		sk.Blocking = v
+	default:
+		// nothing
+	}
+	sk.Accessible = tk.Accessible.UnwrapOr(false)
+	return sk
+}
+
+// Anomaly implements model.MeasurementSummaryKeys.
+func (sk *SummaryKeys) Anomaly() bool {
+	return sk.IsAnomaly
+}

internal/experiment/webconnectivitylte/summary_test.go

@@ -0,0 +1,70 @@
+package webconnectivitylte
+
+import (
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/optional"
+)
+
+func TestSummaryKeys(t *testing.T) {
+	type testcase struct {
+		name             string
+		accessible       optional.Value[bool]
+		blocking         any
+		expectAccessible bool
+		expectBlocking   string
+		expectIsAnomaly  bool
+	}
+
+	cases := []testcase{{
+		name:             "with all nil",
+		accessible:       optional.None[bool](),
+		blocking:         nil,
+		expectAccessible: false,
+		expectBlocking:   "",
+		expectIsAnomaly:  false,
+	}, {
+		name:             "with success",
+		accessible:       optional.Some(true),
+		blocking:         false,
+		expectAccessible: true,
+		expectBlocking:   "",
+		expectIsAnomaly:  false,
+	}, {
+		name:             "with website down",
+		accessible:       optional.Some(false),
+		blocking:         false,
+		expectAccessible: false,
+		expectBlocking:   "",
+		expectIsAnomaly:  false,
+	}, {
+		name:             "with anomaly",
+		accessible:       optional.Some(false),
+		blocking:         "http-failure",
+		expectAccessible: false,
+		expectBlocking:   "http-failure",
+		expectIsAnomaly:  true,
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			tk := &TestKeys{
+				Accessible: tc.accessible,
+				Blocking:   tc.blocking,
+			}
+			sk := tk.MeasurementSummaryKeys().(*SummaryKeys)
+			if sk.Accessible != tc.expectAccessible {
+				t.Fatal("expected", tc.expectAccessible, "got", sk.Accessible)
+			}
+			if sk.Blocking != tc.expectBlocking {
+				t.Fatal("expected", tc.expectBlocking, "got", sk.Blocking)
+			}
+			if sk.IsAnomaly != tc.expectIsAnomaly {
+				t.Fatal("expected", tc.expectIsAnomaly, "got", sk.IsAnomaly)
+			}
+			if sk.Anomaly() != sk.IsAnomaly {
+				t.Fatal("expected Anomaly() to equal IsAnomaly")
+			}
+		})
+	}
+}

internal/experiment/webconnectivitylte/testkeys.go

@@ -127,7 +127,7 @@ type TestKeys struct {
 
 	// Accessible indicates whether the resource is accessible. Possible
 	// values for this field are: nil, true, and false.
-	Accessible any `json:"accessible"`
+	Accessible optional.Value[bool] `json:"accessible"`
 
 	// fundamentalFailure indicates that some fundamental error occurred
 	// in a background task. A fundamental error is something like a programmer
@@ -368,7 +368,7 @@ func NewTestKeys() *TestKeys {
 		StatusCodeMatch:       optional.None[bool](),
 		TitleMatch:            optional.None[bool](),
 		Blocking:              nil,
-		Accessible:            nil,
+		Accessible:            optional.None[bool](),
 		ControlRequest:        nil,
 		fundamentalFailure:    nil,
 		mu:                    &sync.Mutex{},