Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(webconnectivitylte): wire-in SummaryKeys #1493

Merged
merged 2 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/database/actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 6 additions & 6 deletions internal/experiment/webconnectivitylte/analysisclassic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]()
}
}

Expand All @@ -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.
Expand All @@ -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)
}
}

Expand Down
32 changes: 32 additions & 0 deletions internal/experiment/webconnectivitylte/summary.go
Original file line number Diff line number Diff line change
@@ -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
}
70 changes: 70 additions & 0 deletions internal/experiment/webconnectivitylte/summary_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
})
}
}
4 changes: 2 additions & 2 deletions internal/experiment/webconnectivitylte/testkeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{},
Expand Down
Loading