From 11b19e33bfe0fdf6789e0ddf37736a49babb1fa2 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 16 Jan 2024 17:15:25 +0100 Subject: [PATCH 1/5] chore(minipipeline): increase code coverage KTLO work as part of https://github.com/ooni/probe/issues/2640 --- internal/minipipeline/analysis.go | 49 ++- internal/minipipeline/analysis_test.go | 483 ++++++++++++++++++++++ internal/minipipeline/httpdiff_test.go | 34 ++ internal/minipipeline/observation_test.go | 49 +++ internal/minipipeline/utils_test.go | 96 +++++ 5 files changed, 696 insertions(+), 15 deletions(-) create mode 100644 internal/minipipeline/analysis_test.go create mode 100644 internal/minipipeline/httpdiff_test.go create mode 100644 internal/minipipeline/utils_test.go diff --git a/internal/minipipeline/analysis.go b/internal/minipipeline/analysis.go index 250157dd75..2f4a3464be 100644 --- a/internal/minipipeline/analysis.go +++ b/internal/minipipeline/analysis.go @@ -587,59 +587,73 @@ func (wa *WebAnalysis) httpHandleFinalResponse(obs *WebObservation) { wa.httpDiffTitleDifferentLongWords(obs) } -func (wa *WebAnalysis) httpDiffBodyProportionFactor(obs *WebObservation) { +// httpDiffBodyProportionFactor computes the body proportion factor. +// +// The return value--used for testing--is zero on success and negative in case of failure. +func (wa *WebAnalysis) httpDiffBodyProportionFactor(obs *WebObservation) int64 { // we should only perform the comparison for a final response if !obs.HTTPResponseIsFinal.UnwrapOr(false) { - return + return -1 } // we need a valid body length and the body must not be truncated measurement := obs.HTTPResponseBodyLength.UnwrapOr(0) - if measurement <= 0 || obs.HTTPResponseBodyIsTruncated.UnwrapOr(true) { - return + if measurement <= 0 { + return -2 + } + if obs.HTTPResponseBodyIsTruncated.UnwrapOr(true) { + return -3 } // we also need a valid control body length control := obs.ControlHTTPResponseBodyLength.UnwrapOr(0) if control <= 0 { - return + return -4 } // compute the body proportion factor and update the state proportion := ComputeHTTPDiffBodyProportionFactor(measurement, control) wa.HTTPFinalResponseDiffBodyProportionFactor = optional.Some(proportion) + return 0 } -func (wa *WebAnalysis) httpDiffStatusCodeMatch(obs *WebObservation) { +// httpDiffStatusCodeMatch computes whether the status code matches. +// +// The return value--used for testing--is zero on success and negative in case of failure. +func (wa *WebAnalysis) httpDiffStatusCodeMatch(obs *WebObservation) int64 { // we should only perform the comparison for a final response if !obs.HTTPResponseIsFinal.UnwrapOr(false) { - return + return -1 } // we need a positive status code for both measurement := obs.HTTPResponseStatusCode.UnwrapOr(0) if measurement <= 0 { - return + return -2 } control := obs.ControlHTTPResponseStatusCode.UnwrapOr(0) if control <= 0 { - return + return -3 } // update state wa.HTTPFinalResponseDiffStatusCodeMatch = ComputeHTTPDiffStatusCodeMatch(measurement, control) + return 0 } -func (wa *WebAnalysis) httpDiffUncommonHeadersIntersection(obs *WebObservation) { +// httpDiffUncommonHeadersIntersection computes the uncommon headers intersection. +// +// The return value--used for testing--is negative in case of failure and zero or positive otherwise. +func (wa *WebAnalysis) httpDiffUncommonHeadersIntersection(obs *WebObservation) int64 { // we should only perform the comparison for a final response if !obs.HTTPResponseIsFinal.UnwrapOr(false) { - return + return -1 } // We should only perform the comparison if we have valid control data. Because // the headers could legitimately be empty, let's use the status code here. if obs.ControlHTTPResponseStatusCode.UnwrapOr(0) <= 0 { - return + return -2 } // Implementation note: here we need to continue running when either @@ -651,18 +665,22 @@ func (wa *WebAnalysis) httpDiffUncommonHeadersIntersection(obs *WebObservation) state := ComputeHTTPDiffUncommonHeadersIntersection(measurement, control) wa.HTTPFinalResponseDiffUncommonHeadersIntersection = optional.Some(state) + return int64(len(state)) } -func (wa *WebAnalysis) httpDiffTitleDifferentLongWords(obs *WebObservation) { +// httpDiffTitleDifferentLongWords computes the different long words. +// +// The return value--used for testing--is negative in case of failure and zero or positive otherwise. +func (wa *WebAnalysis) httpDiffTitleDifferentLongWords(obs *WebObservation) int64 { // we should only perform the comparison for a final response if !obs.HTTPResponseIsFinal.UnwrapOr(false) { - return + return -1 } // We should only perform the comparison if we have valid control data. Because // the title could legitimately be empty, let's use the status code here. if obs.ControlHTTPResponseStatusCode.UnwrapOr(0) <= 0 { - return + return -2 } measurement := obs.HTTPResponseTitle.UnwrapOr("") @@ -671,4 +689,5 @@ func (wa *WebAnalysis) httpDiffTitleDifferentLongWords(obs *WebObservation) { state := ComputeHTTPDiffTitleDifferentLongWords(measurement, control) wa.HTTPFinalResponseDiffTitleDifferentLongWords = optional.Some(state) + return int64(len(state)) } diff --git a/internal/minipipeline/analysis_test.go b/internal/minipipeline/analysis_test.go new file mode 100644 index 0000000000..158fe56a03 --- /dev/null +++ b/internal/minipipeline/analysis_test.go @@ -0,0 +1,483 @@ +package minipipeline + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/optional" +) + +func TestHTTPDiffBodyProportionFactor(t *testing.T) { + type testcase struct { + name string + ControlHTTPResponseBodyLength optional.Value[int64] + HTTPResponseIsFinal optional.Value[bool] + HTTPResponseBodyLength optional.Value[int64] + HTTPResponseBodyIsTruncated optional.Value[bool] + ExpectReturnValue int64 + ExpectBodyProportionFactor optional.Value[float64] + } + + allcases := []testcase{{ + name: "with missing information on whether the WebObservation is final", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.None[bool](), + HTTPResponseBodyLength: optional.None[int64](), + HTTPResponseBodyIsTruncated: optional.None[bool](), + ExpectReturnValue: -1, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with non-final WebObservation", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some[bool](false), + HTTPResponseBodyLength: optional.None[int64](), + HTTPResponseBodyIsTruncated: optional.None[bool](), + ExpectReturnValue: -1, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with missing response body length", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.None[int64](), + HTTPResponseBodyIsTruncated: optional.None[bool](), + ExpectReturnValue: -2, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with response body length being negative", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](-1), + HTTPResponseBodyIsTruncated: optional.None[bool](), + ExpectReturnValue: -2, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with response body length being zero", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](0), + HTTPResponseBodyIsTruncated: optional.None[bool](), + ExpectReturnValue: -2, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with no information on whether the body is truncated", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](1024), + HTTPResponseBodyIsTruncated: optional.None[bool](), + ExpectReturnValue: -3, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with truncated response body", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](1024), + HTTPResponseBodyIsTruncated: optional.Some(true), + ExpectReturnValue: -3, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with missing control response body length", + ControlHTTPResponseBodyLength: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](1024), + HTTPResponseBodyIsTruncated: optional.Some(false), + ExpectReturnValue: -4, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with control response body length being negative", + ControlHTTPResponseBodyLength: optional.Some[int64](-1), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](1024), + HTTPResponseBodyIsTruncated: optional.Some(false), + ExpectReturnValue: -4, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "with control response body length being zero", + ControlHTTPResponseBodyLength: optional.Some[int64](0), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](1024), + HTTPResponseBodyIsTruncated: optional.Some(false), + ExpectReturnValue: -4, + ExpectBodyProportionFactor: optional.None[float64](), + }, { + name: "successful case", + ControlHTTPResponseBodyLength: optional.Some[int64](2048), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseBodyLength: optional.Some[int64](1024), + HTTPResponseBodyIsTruncated: optional.Some[bool](false), + ExpectReturnValue: 0, + ExpectBodyProportionFactor: optional.Some(0.5), + }} + + for _, tc := range allcases { + t.Run(tc.name, func(t *testing.T) { + obs := &WebObservation{ + HTTPResponseBodyLength: tc.HTTPResponseBodyLength, + HTTPResponseBodyIsTruncated: tc.HTTPResponseBodyIsTruncated, + HTTPResponseIsFinal: tc.HTTPResponseIsFinal, + ControlHTTPResponseBodyLength: tc.ControlHTTPResponseBodyLength, + } + + wa := &WebAnalysis{} + retval := wa.httpDiffBodyProportionFactor(obs) + if diff := cmp.Diff(tc.ExpectReturnValue, retval); diff != "" { + t.Fatal(diff) + } + + result := wa.HTTPFinalResponseDiffBodyProportionFactor + switch { + case tc.ExpectBodyProportionFactor.IsNone() && result.IsNone(): + // all good here + case tc.ExpectBodyProportionFactor.IsNone() && !result.IsNone(): + t.Fatal("expected none, got", result.Unwrap()) + case !tc.ExpectBodyProportionFactor.IsNone() && result.IsNone(): + t.Fatal("expected", tc.ExpectBodyProportionFactor.Unwrap(), "got none") + case !tc.ExpectBodyProportionFactor.IsNone() && !result.IsNone(): + if diff := cmp.Diff(tc.ExpectBodyProportionFactor.Unwrap(), result.Unwrap()); diff != "" { + t.Fatal(diff) + } + } + }) + } +} + +func TestHTTPDiffStatusCodeMatch(t *testing.T) { + type testcase struct { + name string + ControlHTTPResponseStatusCode optional.Value[int64] + HTTPResponseIsFinal optional.Value[bool] + HTTPResponseStatusCode optional.Value[int64] + ExpectReturnValue int64 + ExpectStatusCodeMatch optional.Value[bool] + } + + allcases := []testcase{{ + name: "with missing information on whether the WebObservation is final", + ControlHTTPResponseStatusCode: optional.None[int64](), + HTTPResponseIsFinal: optional.None[bool](), + HTTPResponseStatusCode: optional.None[int64](), + ExpectReturnValue: -1, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with non-final WebObservation", + ControlHTTPResponseStatusCode: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(false), + HTTPResponseStatusCode: optional.None[int64](), + ExpectReturnValue: -1, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with missing response status code", + ControlHTTPResponseStatusCode: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.None[int64](), + ExpectReturnValue: -2, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with negative response status code", + ControlHTTPResponseStatusCode: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](-1), + ExpectReturnValue: -2, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with zero response status code", + ControlHTTPResponseStatusCode: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](0), + ExpectReturnValue: -2, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with missing control response status code", + ControlHTTPResponseStatusCode: optional.None[int64](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](200), + ExpectReturnValue: -3, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with negative control response status code", + ControlHTTPResponseStatusCode: optional.Some[int64](-1), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](200), + ExpectReturnValue: -3, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "with zero control response status code", + ControlHTTPResponseStatusCode: optional.Some[int64](0), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](200), + ExpectReturnValue: -3, + ExpectStatusCodeMatch: optional.None[bool](), + }, { + name: "successful case with match", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](200), + ExpectReturnValue: 0, + ExpectStatusCodeMatch: optional.Some[bool](true), + }, { + name: "successful case with mismatch", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](500), + ExpectReturnValue: 0, + ExpectStatusCodeMatch: optional.Some[bool](false), + }, { + name: "successful case with mismatch and 5xx control", + ControlHTTPResponseStatusCode: optional.Some[int64](500), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseStatusCode: optional.Some[int64](403), + ExpectReturnValue: 0, + ExpectStatusCodeMatch: optional.None[bool](), + }} + + for _, tc := range allcases { + t.Run(tc.name, func(t *testing.T) { + obs := &WebObservation{ + HTTPResponseStatusCode: tc.HTTPResponseStatusCode, + HTTPResponseIsFinal: tc.HTTPResponseIsFinal, + ControlHTTPResponseStatusCode: tc.ControlHTTPResponseStatusCode, + } + + wa := &WebAnalysis{} + retval := wa.httpDiffStatusCodeMatch(obs) + if diff := cmp.Diff(tc.ExpectReturnValue, retval); diff != "" { + t.Fatal(diff) + } + + result := wa.HTTPFinalResponseDiffStatusCodeMatch + switch { + case tc.ExpectStatusCodeMatch.IsNone() && result.IsNone(): + // all good here + case tc.ExpectStatusCodeMatch.IsNone() && !result.IsNone(): + t.Fatal("expected none, got", result.Unwrap()) + case !tc.ExpectStatusCodeMatch.IsNone() && result.IsNone(): + t.Fatal("expected", tc.ExpectStatusCodeMatch.Unwrap(), "got none") + case !tc.ExpectStatusCodeMatch.IsNone() && !result.IsNone(): + if diff := cmp.Diff(tc.ExpectStatusCodeMatch.Unwrap(), result.Unwrap()); diff != "" { + t.Fatal(diff) + } + } + }) + } +} + +func TestHTTPDiffUncommonHeadersIntersection(t *testing.T) { + type testcase struct { + name string + ControlHTTPResponseStatusCode optional.Value[int64] + ControlHTTPResponseHeaderKeys optional.Value[map[string]bool] + HTTPResponseIsFinal optional.Value[bool] + HTTPResponseHeaderKeys optional.Value[map[string]bool] + ExpectReturnValue int64 + ExpectHeadersIntersection optional.Value[map[string]bool] + } + + allcases := []testcase{{ + name: "when we don't know whether the WebObservation is final", + ControlHTTPResponseStatusCode: optional.None[int64](), + ControlHTTPResponseHeaderKeys: optional.None[map[string]bool](), + HTTPResponseIsFinal: optional.None[bool](), + HTTPResponseHeaderKeys: optional.None[map[string]bool](), + ExpectReturnValue: -1, + ExpectHeadersIntersection: optional.None[map[string]bool](), + }, { + name: "when the WebObservation is not final", + ControlHTTPResponseStatusCode: optional.None[int64](), + ControlHTTPResponseHeaderKeys: optional.None[map[string]bool](), + HTTPResponseIsFinal: optional.Some[bool](false), + HTTPResponseHeaderKeys: optional.None[map[string]bool](), + ExpectReturnValue: -1, + ExpectHeadersIntersection: optional.None[map[string]bool](), + }, { + name: "when we don't know the control status code", + ControlHTTPResponseStatusCode: optional.None[int64](), + ControlHTTPResponseHeaderKeys: optional.None[map[string]bool](), + HTTPResponseIsFinal: optional.Some[bool](true), + HTTPResponseHeaderKeys: optional.None[map[string]bool](), + ExpectReturnValue: -2, + ExpectHeadersIntersection: optional.None[map[string]bool](), + }, { + name: "when the control status code is negative", + ControlHTTPResponseStatusCode: optional.Some[int64](-1), + ControlHTTPResponseHeaderKeys: optional.None[map[string]bool](), + HTTPResponseIsFinal: optional.Some[bool](true), + HTTPResponseHeaderKeys: optional.None[map[string]bool](), + ExpectReturnValue: -2, + ExpectHeadersIntersection: optional.None[map[string]bool](), + }, { + name: "when the control status code is zero", + ControlHTTPResponseStatusCode: optional.Some[int64](0), + ControlHTTPResponseHeaderKeys: optional.None[map[string]bool](), + HTTPResponseIsFinal: optional.Some[bool](true), + HTTPResponseHeaderKeys: optional.None[map[string]bool](), + ExpectReturnValue: -2, + ExpectHeadersIntersection: optional.None[map[string]bool](), + }, { + name: "with missing headers information", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + ControlHTTPResponseHeaderKeys: optional.None[map[string]bool](), + HTTPResponseIsFinal: optional.Some[bool](true), + HTTPResponseHeaderKeys: optional.None[map[string]bool](), + ExpectReturnValue: 0, + ExpectHeadersIntersection: optional.Some(map[string]bool{}), + }, { + name: "with no headers intersection", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + ControlHTTPResponseHeaderKeys: optional.Some(map[string]bool{"A": true}), // uncommon header + HTTPResponseIsFinal: optional.Some[bool](true), + HTTPResponseHeaderKeys: optional.Some(map[string]bool{"B": true}), // uncommon header + ExpectReturnValue: 0, + ExpectHeadersIntersection: optional.Some(map[string]bool{}), + }, { + name: "with headers intersection", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + ControlHTTPResponseHeaderKeys: optional.Some(map[string]bool{"C": true}), // uncommon header + HTTPResponseIsFinal: optional.Some[bool](true), + HTTPResponseHeaderKeys: optional.Some(map[string]bool{"C": true}), // uncommon header + ExpectReturnValue: 1, + ExpectHeadersIntersection: optional.Some(map[string]bool{"c": true}), // lowercase b/c it's normalized + }} + + for _, tc := range allcases { + t.Run(tc.name, func(t *testing.T) { + obs := &WebObservation{ + ControlHTTPResponseStatusCode: tc.ControlHTTPResponseStatusCode, + ControlHTTPResponseHeadersKeys: tc.ControlHTTPResponseHeaderKeys, + HTTPResponseIsFinal: tc.HTTPResponseIsFinal, + HTTPResponseHeadersKeys: tc.HTTPResponseHeaderKeys, + } + + wa := &WebAnalysis{} + retval := wa.httpDiffUncommonHeadersIntersection(obs) + if diff := cmp.Diff(tc.ExpectReturnValue, retval); diff != "" { + t.Fatal(diff) + } + + result := wa.HTTPFinalResponseDiffUncommonHeadersIntersection + switch { + case tc.ExpectHeadersIntersection.IsNone() && result.IsNone(): + // all good here + case tc.ExpectHeadersIntersection.IsNone() && !result.IsNone(): + t.Fatal("expected none, got", result.Unwrap()) + case !tc.ExpectHeadersIntersection.IsNone() && result.IsNone(): + t.Fatal("expected", tc.ExpectHeadersIntersection.Unwrap(), "got none") + case !tc.ExpectHeadersIntersection.IsNone() && !result.IsNone(): + if diff := cmp.Diff(tc.ExpectHeadersIntersection.Unwrap(), result.Unwrap()); diff != "" { + t.Fatal(diff) + } + } + }) + } +} + +func TestHTTPDiffTitleDifferentLongWords(t *testing.T) { + type testcase struct { + name string + ControlHTTPResponseStatusCode optional.Value[int64] + ControlHTTPResponseTitle optional.Value[string] + HTTPResponseIsFinal optional.Value[bool] + HTTPResponseTitle optional.Value[string] + ExpectReturnValue int64 + ExpectDifferentLongWords optional.Value[map[string]bool] + } + + allcases := []testcase{{ + name: "with no information on whether the observation is final", + ControlHTTPResponseStatusCode: optional.None[int64](), + ControlHTTPResponseTitle: optional.None[string](), + HTTPResponseIsFinal: optional.None[bool](), + HTTPResponseTitle: optional.None[string](), + ExpectReturnValue: -1, + ExpectDifferentLongWords: optional.None[map[string]bool](), + }, { + name: "with non-final response", + ControlHTTPResponseStatusCode: optional.None[int64](), + ControlHTTPResponseTitle: optional.None[string](), + HTTPResponseIsFinal: optional.Some(false), + HTTPResponseTitle: optional.None[string](), + ExpectReturnValue: -1, + ExpectDifferentLongWords: optional.None[map[string]bool](), + }, { + name: "with no information on control status code", + ControlHTTPResponseStatusCode: optional.None[int64](), + ControlHTTPResponseTitle: optional.None[string](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseTitle: optional.None[string](), + ExpectReturnValue: -2, + ExpectDifferentLongWords: optional.None[map[string]bool](), + }, { + name: "with negative control status code", + ControlHTTPResponseStatusCode: optional.Some[int64](-1), + ControlHTTPResponseTitle: optional.None[string](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseTitle: optional.None[string](), + ExpectReturnValue: -2, + ExpectDifferentLongWords: optional.None[map[string]bool](), + }, { + name: "with zero control status code", + ControlHTTPResponseStatusCode: optional.Some[int64](0), + ControlHTTPResponseTitle: optional.None[string](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseTitle: optional.None[string](), + ExpectReturnValue: -2, + ExpectDifferentLongWords: optional.None[map[string]bool](), + }, { + name: "with no titles", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + ControlHTTPResponseTitle: optional.None[string](), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseTitle: optional.None[string](), + ExpectReturnValue: 0, + ExpectDifferentLongWords: optional.Some(map[string]bool{}), + }, { + name: "with no different long words", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + ControlHTTPResponseTitle: optional.Some("Antani Mascetti Melandri"), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseTitle: optional.Some("Mascetti Melandri Antani"), + ExpectReturnValue: 0, + ExpectDifferentLongWords: optional.Some(map[string]bool{}), + }, { + name: "with different long words", + ControlHTTPResponseStatusCode: optional.Some[int64](200), + ControlHTTPResponseTitle: optional.Some("Antani Mascetti Melandri"), + HTTPResponseIsFinal: optional.Some(true), + HTTPResponseTitle: optional.Some("Forbidden Mascetti"), + ExpectReturnValue: 3, + ExpectDifferentLongWords: optional.Some(map[string]bool{ + "antani": true, + "forbidden": true, + "melandri": true, + }), + }} + + for _, tc := range allcases { + t.Run(tc.name, func(t *testing.T) { + obs := &WebObservation{ + ControlHTTPResponseStatusCode: tc.ControlHTTPResponseStatusCode, + ControlHTTPResponseTitle: tc.ControlHTTPResponseTitle, + HTTPResponseIsFinal: tc.HTTPResponseIsFinal, + HTTPResponseTitle: tc.HTTPResponseTitle, + } + + wa := &WebAnalysis{} + retval := wa.httpDiffTitleDifferentLongWords(obs) + if diff := cmp.Diff(tc.ExpectReturnValue, retval); diff != "" { + t.Fatal(diff) + } + + result := wa.HTTPFinalResponseDiffTitleDifferentLongWords + switch { + case tc.ExpectDifferentLongWords.IsNone() && result.IsNone(): + // all good here + case tc.ExpectDifferentLongWords.IsNone() && !result.IsNone(): + t.Fatal("expected none, got", result.Unwrap()) + case !tc.ExpectDifferentLongWords.IsNone() && result.IsNone(): + t.Fatal("expected", tc.ExpectDifferentLongWords.Unwrap(), "got none") + case !tc.ExpectDifferentLongWords.IsNone() && !result.IsNone(): + if diff := cmp.Diff(tc.ExpectDifferentLongWords.Unwrap(), result.Unwrap()); diff != "" { + t.Fatal(diff) + } + } + }) + } +} diff --git a/internal/minipipeline/httpdiff_test.go b/internal/minipipeline/httpdiff_test.go new file mode 100644 index 0000000000..76f3e98189 --- /dev/null +++ b/internal/minipipeline/httpdiff_test.go @@ -0,0 +1,34 @@ +package minipipeline + +import "testing" + +func TestComputeHTTPDiffStatusCodeMatch(t *testing.T) { + t.Run( + "when the control status code is not 2xx and the measurement is different from the control", + func(t *testing.T) { + result := ComputeHTTPDiffStatusCodeMatch(200, 500) + if !result.IsNone() { + t.Fatal("should be none") + } + }) + + t.Run("when both are 500", func(t *testing.T) { + result := ComputeHTTPDiffStatusCodeMatch(500, 500) + if result.IsNone() { + t.Fatal("should not be none") + } + if result.Unwrap() != true { + t.Fatal("result should be true") + } + }) + + t.Run("when both are 200", func(t *testing.T) { + result := ComputeHTTPDiffStatusCodeMatch(200, 200) + if result.IsNone() { + t.Fatal("should not be none") + } + if result.Unwrap() != true { + t.Fatal("result should be true") + } + }) +} diff --git a/internal/minipipeline/observation_test.go b/internal/minipipeline/observation_test.go index c39528d226..212912ee88 100644 --- a/internal/minipipeline/observation_test.go +++ b/internal/minipipeline/observation_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/optional" ) @@ -154,4 +155,52 @@ func TestWebObservationsContainerIngestControlMessages(t *testing.T) { t.Fatal("ControlTLSHandshakeFailure should be none") } }) + + t.Run("we correctly handle an inconsisten control DNS lookup result", func(t *testing.T) { + container := &WebObservationsContainer{ + DNSLookupFailures: []*WebObservation{}, + KnownTCPEndpoints: map[int64]*WebObservation{ + 1: { + DNSDomain: optional.Some("dns.google.com"), + IPAddress: optional.Some("8.8.8.8"), + EndpointTransactionID: optional.Some(int64(1)), + EndpointPort: optional.Some("443"), + EndpointAddress: optional.Some("8.8.8.8:443"), + TLSServerName: optional.Some("dns.google.com"), + }, + }, + knownIPAddresses: map[string]*WebObservation{}, + } + + thRequest := &model.THRequest{ + HTTPRequest: "https://dns.google.com/", + } + + // Note: the following entry is completely unrealistic but it + // is here to show that, even with a malformed entry, we are still + // able to avoid producing a wrong result for an address. + // + // Specifically, here the DNS failure should take precedence over + // the resolved addresses and ControlDNSResolvedAddr should be empty. + thResponse := &model.THResponse{ + DNS: model.THDNSResult{ + Failure: (func() *string { + v := netxlite.FailureGenericTimeoutError + return &v + })(), + Addrs: []string{"8.8.8.8"}, + ASNs: []int64{}, + }, + } + + if err := container.IngestControlMessages(thRequest, thResponse); err != nil { + t.Fatal(err) + } + + entry := container.KnownTCPEndpoints[1] + + if !entry.ControlDNSResolvedAddrs.IsNone() { + t.Fatal("ControlDNSResolvedAddrs should be none") + } + }) } diff --git a/internal/minipipeline/utils_test.go b/internal/minipipeline/utils_test.go new file mode 100644 index 0000000000..31bbf07db9 --- /dev/null +++ b/internal/minipipeline/utils_test.go @@ -0,0 +1,96 @@ +package minipipeline + +import "testing" + +func TestUtilsExtractTagDepth(t *testing.T) { + t.Run("with nil tags list", func(t *testing.T) { + result := utilsExtractTagDepth(nil) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("with empty tags list", func(t *testing.T) { + result := utilsExtractTagDepth([]string{}) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("with missing depth=123 tag", func(t *testing.T) { + result := utilsExtractTagDepth([]string{"a", "b", "c", "d"}) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("with depth=NotANumber tag", func(t *testing.T) { + result := utilsExtractTagDepth([]string{"depth=NotANumber"}) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("we only return the first entry", func(t *testing.T) { + result := utilsExtractTagDepth([]string{"depth=10", "depth=12"}) + if result.IsNone() { + t.Fatal("expected not none") + } + if value := result.Unwrap(); value != 10 { + t.Fatal("expected 10, got", value) + } + }) +} + +func TestUtilsTagFetchBody(t *testing.T) { + t.Run("with nil tags list", func(t *testing.T) { + result := utilsExtractTagFetchBody(nil) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("with empty tags list", func(t *testing.T) { + result := utilsExtractTagFetchBody([]string{}) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("with missing feth_body=BOOL tag", func(t *testing.T) { + result := utilsExtractTagFetchBody([]string{"a", "b", "c", "d"}) + if !result.IsNone() { + t.Fatal("expected none") + } + }) + + t.Run("with fetch_body=true tag", func(t *testing.T) { + result := utilsExtractTagFetchBody([]string{"fetch_body=false"}) + if result.IsNone() { + t.Fatal("expected not none") + } + if value := result.Unwrap(); value != false { + t.Fatal("expected false, got", value) + } + }) + + t.Run("with fetch_body=false tag", func(t *testing.T) { + result := utilsExtractTagFetchBody([]string{"fetch_body=true"}) + if result.IsNone() { + t.Fatal("expected not none") + } + if value := result.Unwrap(); value != true { + t.Fatal("expected true, got", value) + } + }) + + t.Run("we only return the first entry", func(t *testing.T) { + result := utilsExtractTagFetchBody([]string{"fetch_body=false", "fetch_body=true"}) + if result.IsNone() { + t.Fatal("expected not none") + } + if value := result.Unwrap(); value != false { + t.Fatal("expected false, got", value) + } + }) +} From 2149d3c8f5d0b36ef329301f88279ba1ddabac05 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 16 Jan 2024 17:19:42 +0100 Subject: [PATCH 2/5] Update internal/minipipeline/observation_test.go --- internal/minipipeline/observation_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/minipipeline/observation_test.go b/internal/minipipeline/observation_test.go index 212912ee88..93d3e5e74b 100644 --- a/internal/minipipeline/observation_test.go +++ b/internal/minipipeline/observation_test.go @@ -156,7 +156,7 @@ func TestWebObservationsContainerIngestControlMessages(t *testing.T) { } }) - t.Run("we correctly handle an inconsisten control DNS lookup result", func(t *testing.T) { + t.Run("we correctly handle an inconsistent control DNS lookup result", func(t *testing.T) { container := &WebObservationsContainer{ DNSLookupFailures: []*WebObservation{}, KnownTCPEndpoints: map[int64]*WebObservation{ From e8bd2ea89075a4d1b3917637081b9b8b922e0406 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 16 Jan 2024 17:22:20 +0100 Subject: [PATCH 3/5] x --- internal/minipipeline/utils_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/minipipeline/utils_test.go b/internal/minipipeline/utils_test.go index 31bbf07db9..1819896579 100644 --- a/internal/minipipeline/utils_test.go +++ b/internal/minipipeline/utils_test.go @@ -64,7 +64,7 @@ func TestUtilsTagFetchBody(t *testing.T) { } }) - t.Run("with fetch_body=true tag", func(t *testing.T) { + t.Run("with fetch_body=false tag", func(t *testing.T) { result := utilsExtractTagFetchBody([]string{"fetch_body=false"}) if result.IsNone() { t.Fatal("expected not none") @@ -74,7 +74,7 @@ func TestUtilsTagFetchBody(t *testing.T) { } }) - t.Run("with fetch_body=false tag", func(t *testing.T) { + t.Run("with fetch_body=true tag", func(t *testing.T) { result := utilsExtractTagFetchBody([]string{"fetch_body=true"}) if result.IsNone() { t.Fatal("expected not none") From 3808e572d2778671e2593dddce5ba7341c54fdd0 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 16 Jan 2024 17:23:59 +0100 Subject: [PATCH 4/5] x --- internal/minipipeline/utils.go | 14 ++++++++------ internal/minipipeline/utils_test.go | 10 +++++----- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/internal/minipipeline/utils.go b/internal/minipipeline/utils.go index 96a8a75efb..87ed880d2c 100644 --- a/internal/minipipeline/utils.go +++ b/internal/minipipeline/utils.go @@ -74,7 +74,8 @@ func utilsEngineIsGetaddrinfo(engine optional.Value[string]) bool { } } -func utilsExtractTagDepth(tags []string) optional.Value[int64] { +func utilsExtractTagDepth(tags []string) (result optional.Value[int64]) { + result = optional.None[int64]() for _, tag := range tags { if !strings.HasPrefix(tag, "depth=") { continue @@ -84,20 +85,21 @@ func utilsExtractTagDepth(tags []string) optional.Value[int64] { if err != nil { continue } - return optional.Some(value) + result = optional.Some(value) } - return optional.None[int64]() + return } -func utilsExtractTagFetchBody(tags []string) optional.Value[bool] { +func utilsExtractTagFetchBody(tags []string) (result optional.Value[bool]) { + result = optional.None[bool]() for _, tag := range tags { if !strings.HasPrefix(tag, "fetch_body=") { continue } tag = strings.TrimPrefix(tag, "fetch_body=") - return optional.Some(tag == "true") + result = optional.Some(tag == "true") } - return optional.None[bool]() + return } func utilsDNSLookupFailureIsDNSNoAnswerForAAAA(obs *WebObservation) bool { diff --git a/internal/minipipeline/utils_test.go b/internal/minipipeline/utils_test.go index 1819896579..d5194ed319 100644 --- a/internal/minipipeline/utils_test.go +++ b/internal/minipipeline/utils_test.go @@ -31,13 +31,13 @@ func TestUtilsExtractTagDepth(t *testing.T) { } }) - t.Run("we only return the first entry", func(t *testing.T) { + t.Run("we return the last entry", func(t *testing.T) { result := utilsExtractTagDepth([]string{"depth=10", "depth=12"}) if result.IsNone() { t.Fatal("expected not none") } - if value := result.Unwrap(); value != 10 { - t.Fatal("expected 10, got", value) + if value := result.Unwrap(); value != 12 { + t.Fatal("expected 12, got", value) } }) } @@ -84,12 +84,12 @@ func TestUtilsTagFetchBody(t *testing.T) { } }) - t.Run("we only return the first entry", func(t *testing.T) { + t.Run("we return the last entry", func(t *testing.T) { result := utilsExtractTagFetchBody([]string{"fetch_body=false", "fetch_body=true"}) if result.IsNone() { t.Fatal("expected not none") } - if value := result.Unwrap(); value != false { + if value := result.Unwrap(); value != true { t.Fatal("expected false, got", value) } }) From 7b5c7eab83116f01e97c5c9d8ba3266c3f093de9 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 16 Jan 2024 17:29:31 +0100 Subject: [PATCH 5/5] x --- internal/minipipeline/analysis_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/minipipeline/analysis_test.go b/internal/minipipeline/analysis_test.go index 158fe56a03..b9717164a0 100644 --- a/internal/minipipeline/analysis_test.go +++ b/internal/minipipeline/analysis_test.go @@ -444,6 +444,7 @@ func TestHTTPDiffTitleDifferentLongWords(t *testing.T) { HTTPResponseTitle: optional.Some("Forbidden Mascetti"), ExpectReturnValue: 3, ExpectDifferentLongWords: optional.Some(map[string]bool{ + // note: names normalized to lowercase "antani": true, "forbidden": true, "melandri": true,