fix(minipipeline): reduce generated test cases churn

We add dns.nextdns.io to the QA suite, such that we don't get a
wrong NXDOMAIN when trying to use it as the resolver.

We optionally sort the test keys to ensure there's less churn in
diffs produced when regnerating minipipeline test data.

We fix ./script/updateminipipeline.bash to use ./script/go.bash
for building as opposed to using go.

Closes ooni/probe#2677

internal/cmd/qatool/main.go

@@ -135,6 +135,9 @@ func main() {
 	// build the regexp
 	selector := regexp.MustCompile(*runFlag)
 
+	// make sure we produce more predictable observations in output
+	webconnectivitylte.SortObservations.Add(1)
+
 	// select which test cases to run
 	for _, tc := range webconnectivityqa.AllTestCases() {
 		name := "webconnectivitylte/" + tc.Name

internal/experiment/webconnectivitylte/measurer.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity"
 	"github.com/ooni/probe-cli/v3/internal/inputparser"
+	"github.com/ooni/probe-cli/v3/internal/minipipeline"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"golang.org/x/net/publicsuffix"
 )
@@ -144,6 +145,23 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 		}
 	}
 
+	// sort test keys to make output more predictable and avoid churn when generating
+	// minipipeline test cases; see https://github.com/ooni/probe/issues/2677.
+	//
+	// Note that tk.Do53 and tk.DoH are initialized by NewTestKeys so we know they're not nil.
+	//
+	// Note that we MUST NOT sort tk.Requests because its order matters for historical
+	// reasons and we don't wnat to break existing data consumers.
+	if SortObservations.Load() > 0 {
+		tk.Queries = minipipeline.SortDNSLookupResults(tk.Queries)
+		tk.Do53.Queries = minipipeline.SortDNSLookupResults(tk.Do53.Queries)
+		tk.DoH.Queries = minipipeline.SortDNSLookupResults(tk.DoH.Queries)
+		tk.DNSDuplicateResponses = minipipeline.SortDNSLookupResults(tk.DNSDuplicateResponses)
+		tk.NetworkEvents = minipipeline.SortNetworkEvents(tk.NetworkEvents)
+		tk.TCPConnect = minipipeline.SortTCPConnectResults(tk.TCPConnect)
+		tk.TLSHandshakes = minipipeline.SortTLSHandshakeResults(tk.TLSHandshakes)
+	}
+
 	// return whether there was a fundamental failure, which would prevent
 	// the measurement from being submitted to the OONI collector.
 	return tk.fundamentalFailure
@@ -159,3 +177,7 @@ func registerExtensions(m *model.Measurement) {
 	model.ArchivalExtTLSHandshake.AddTo(m)
 	model.ArchivalExtTunnel.AddTo(m)
 }
+
+// SortObservations allows to configure sorting observations when that's needed to
+// reduce churn in the generated JSON files (mainly for minipipeline).
+var SortObservations = &atomic.Int64{}

internal/experiment/webconnectivitylte/qa_test.go

@@ -7,6 +7,7 @@ import (
 )
 
 func TestQA(t *testing.T) {
+	SortObservations.Add(1) // make sure we have predictable observations
 	for _, tc := range webconnectivityqa.AllTestCases() {
 		t.Run(tc.Name, func(t *testing.T) {
 			if (tc.Flags & webconnectivityqa.TestCaseFlagNoLTE) != 0 {

internal/experiment/webconnectivitylte/testkeys.go

@@ -291,13 +291,6 @@ func (tk *TestKeys) WithDNSWhoami(fun func(*DNSWhoamiInfo)) {
 	tk.mu.Unlock()
 }
 
-// SetClientResolver sets the ClientResolver field.
-func (tk *TestKeys) SetClientResolver(value string) {
-	tk.mu.Lock()
-	tk.ClientResolver = value
-	tk.mu.Unlock()
-}
-
 // AppendConnPriorityLogEntry appends an entry to ConnPriorityLog.
 func (tk *TestKeys) AppendConnPriorityLogEntry(entry *ConnPriorityLogEntry) {
 	tk.mu.Lock()

internal/minipipeline/analysis.go

@@ -59,13 +59,15 @@ func NewLinearWebAnalysis(input *WebObservationsContainer) (output []*WebObserva
 		// TagDepth sort at the end of the generated list.
 		if left.TagDepth.UnwrapOr(-1) > right.TagDepth.UnwrapOr(-1) {
 			return true
-		} else if left.TagDepth.UnwrapOr(-1) < right.TagDepth.UnwrapOr(-1) {
+		}
+		if left.TagDepth.UnwrapOr(-1) < right.TagDepth.UnwrapOr(-1) {
 			return false
 		}
 
 		if left.Type > right.Type {
 			return true
-		} else if left.Type < right.Type {
+		}
+		if left.Type < right.Type {
 			return false
 		}
 
@@ -74,7 +76,8 @@ func NewLinearWebAnalysis(input *WebObservationsContainer) (output []*WebObserva
 		const defaultFailureValue = "unknown_failure"
 		if left.Failure.UnwrapOr(defaultFailureValue) < right.Failure.UnwrapOr(defaultFailureValue) {
 			return true
-		} else if left.Failure.UnwrapOr(defaultFailureValue) > right.Failure.UnwrapOr(defaultFailureValue) {
+		}
+		if left.Failure.UnwrapOr(defaultFailureValue) > right.Failure.UnwrapOr(defaultFailureValue) {
 			return false
 		}
 

internal/minipipeline/sorting.go

@@ -0,0 +1,121 @@
+package minipipeline
+
+import (
+	"sort"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+// SortDNSLookupResults sorts and returns a copy of the DNS lookup results to avoid too-much
+// timing dependent churn when generating testcases for the minipipeline.
+func SortDNSLookupResults(inputs []*model.ArchivalDNSLookupResult) (outputs []*model.ArchivalDNSLookupResult) {
+	// copy the original slice
+	outputs = append(outputs, inputs...)
+
+	// sort using complex sorting rule
+	sort.SliceStable(outputs, func(i, j int) bool {
+		left, right := outputs[i], outputs[j]
+
+		// we sort groups by resolver type to avoid the churn caused by parallel runs.
+		if left.Engine < right.Engine {
+			return true
+		}
+		if left.Engine > right.Engine {
+			return false
+		}
+
+		// within the same group, we sort by ascending transaction ID
+		if left.TransactionID < right.TransactionID {
+			return true
+		}
+		if left.TransactionID > right.TransactionID {
+			return false
+		}
+
+		// we want entries that are successful to appear first
+		fsget := func(value *string) string {
+			if value == nil {
+				return ""
+			}
+			return *value
+		}
+		return fsget(left.Failure) < fsget(right.Failure)
+	})
+
+	return
+}
+
+// SortNetworkEvents is like [SortDNSLookupResults] but for network events.
+func SortNetworkEvents(inputs []*model.ArchivalNetworkEvent) (outputs []*model.ArchivalNetworkEvent) {
+	// copy the original slice
+	outputs = append(outputs, inputs...)
+
+	// sort using complex sorting rule
+	sort.SliceStable(outputs, func(i, j int) bool {
+		left, right := outputs[i], outputs[j]
+
+		if left.Address < right.Address {
+			return true
+		}
+		if left.Address > right.Address {
+			return false
+		}
+
+		return left.TransactionID < right.TransactionID
+	})
+
+	return
+}
+
+// SortTCPConnectResults is like [SortDNSLookupResults] but for TCP connect results.
+func SortTCPConnectResults(
+	inputs []*model.ArchivalTCPConnectResult) (outputs []*model.ArchivalTCPConnectResult) {
+	// copy the original slice
+	outputs = append(outputs, inputs...)
+
+	// sort using complex sorting rule
+	sort.SliceStable(outputs, func(i, j int) bool {
+		left, right := outputs[i], outputs[j]
+
+		if left.IP < right.IP {
+			return true
+		}
+		if left.IP > right.IP {
+			return false
+		}
+
+		if left.Port < right.Port {
+			return true
+		}
+		if left.Port > right.Port {
+			return false
+		}
+
+		return left.TransactionID < right.TransactionID
+	})
+
+	return
+}
+
+// SortTLSHandshakeResults is like [SortDNSLookupResults] but for TLS handshake results.
+func SortTLSHandshakeResults(
+	inputs []*model.ArchivalTLSOrQUICHandshakeResult) (outputs []*model.ArchivalTLSOrQUICHandshakeResult) {
+	// copy the original slice
+	outputs = append(outputs, inputs...)
+
+	// sort using complex sorting rule
+	sort.SliceStable(outputs, func(i, j int) bool {
+		left, right := outputs[i], outputs[j]
+
+		if left.Address < right.Address {
+			return true
+		}
+		if left.Address > right.Address {
+			return false
+		}
+
+		return left.TransactionID < right.TransactionID
+	})
+
+	return
+}

internal/minipipeline/sorting_test.go

@@ -0,0 +1,133 @@
+package minipipeline
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+func TestSortDNSLookupResults(t *testing.T) {
+	newfailurestring := func(s string) *string {
+		return &s
+	}
+
+	inputGen := func() []*model.ArchivalDNSLookupResult {
+		return []*model.ArchivalDNSLookupResult{
+			{
+				Engine:          "udp",
+				Failure:         newfailurestring("dns_no_answer"),
+				QueryType:       "AAAA",
+				ResolverAddress: "1.1.1.1:53",
+				TransactionID:   5,
+			},
+			{
+				Engine:          "udp",
+				Failure:         nil,
+				QueryType:       "A",
+				ResolverAddress: "1.1.1.1:53",
+				TransactionID:   5,
+			},
+			{
+				Engine:          "udp",
+				Failure:         newfailurestring("dns_no_answer"),
+				QueryType:       "AAAA",
+				ResolverAddress: "8.8.8.8:53",
+				TransactionID:   3,
+			},
+			{
+				Engine:          "udp",
+				Failure:         nil,
+				QueryType:       "A",
+				ResolverAddress: "8.8.8.8:53",
+				TransactionID:   3,
+			},
+			{
+				Engine:          "doh",
+				Failure:         newfailurestring("dns_no_answer"),
+				QueryType:       "AAAA",
+				ResolverAddress: "https://dns.google/dns-query",
+				TransactionID:   2,
+			},
+			{
+				Engine:          "doh",
+				Failure:         nil,
+				QueryType:       "A",
+				ResolverAddress: "https://dns.google/dns-query",
+				TransactionID:   2,
+			},
+			{
+				Engine:        "getaddrinfo",
+				QueryType:     "ANY",
+				Failure:       nil,
+				TransactionID: 1,
+			},
+		}
+	}
+
+	expect := []*model.ArchivalDNSLookupResult{
+		{
+			Engine:          "doh",
+			Failure:         nil,
+			QueryType:       "A",
+			ResolverAddress: "https://dns.google/dns-query",
+			TransactionID:   2,
+		},
+		{
+			Engine:          "doh",
+			Failure:         newfailurestring("dns_no_answer"),
+			QueryType:       "AAAA",
+			ResolverAddress: "https://dns.google/dns-query",
+			TransactionID:   2,
+		},
+		{
+			Engine:        "getaddrinfo",
+			QueryType:     "ANY",
+			Failure:       nil,
+			TransactionID: 1,
+		},
+		{
+			Engine:          "udp",
+			Failure:         nil,
+			QueryType:       "A",
+			ResolverAddress: "8.8.8.8:53",
+			TransactionID:   3,
+		},
+		{
+			Engine:          "udp",
+			Failure:         newfailurestring("dns_no_answer"),
+			QueryType:       "AAAA",
+			ResolverAddress: "8.8.8.8:53",
+			TransactionID:   3,
+		},
+		{
+			Engine:          "udp",
+			Failure:         nil,
+			QueryType:       "A",
+			ResolverAddress: "1.1.1.1:53",
+			TransactionID:   5,
+		},
+		{
+			Engine:          "udp",
+			Failure:         newfailurestring("dns_no_answer"),
+			QueryType:       "AAAA",
+			ResolverAddress: "1.1.1.1:53",
+			TransactionID:   5,
+		},
+	}
+
+	input := inputGen()
+	output := SortDNSLookupResults(input)
+
+	t.Run("the input should not have mutated", func(t *testing.T) {
+		if diff := cmp.Diff(inputGen(), input); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+
+	t.Run("the output should be consistent with expectations", func(t *testing.T) {
+		if diff := cmp.Diff(expect, output); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+}