Test fixes

- Fix TestTactics to reflect change in 33d508b1
- Fix TestPackedAPIParameters test case
- Add missing error return checks in test inproxy coordinator
- Check for json.Unmarshal errors in tests
- TargetAPIEncoding backwards compatibility in ClientLibrary test

ClientLibrary/clientlib/clientlib_test.go

@@ -26,6 +26,8 @@ import (
 	"os"
 	"testing"
 	"time"
+
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
 )
 
 func TestStartTunnel(t *testing.T) {
@@ -56,7 +58,18 @@ func TestStartTunnel(t *testing.T) {
 	if err != nil {
 		t.Fatalf("json.Unmarshal failed: %v", err)
 	}
+
+	// Use the legacy encoding to both exercise that case, and facilitate a
+	// gradual network upgrade to new encoding support.
+	config["TargetAPIEncoding"] = protocol.PSIPHON_API_ENCODING_JSON
+
+	configJSON, err = json.Marshal(config)
+	if err != nil {
+		t.Fatalf("json.Marshal failed: %v", err)
+	}
+
 	config["DisableRemoteServerListFetcher"] = true
+
 	configJSONNoFetcher, err := json.Marshal(config)
 	if err != nil {
 		t.Fatalf("json.Marshal failed: %v", err)

psiphon/common/inproxy/coordinator_test.go

@@ -326,7 +326,10 @@ func (t *testWebRTCDialCoordinator) UDPListen(_ context.Context) (net.PacketConn
 	t.mutex.Lock()
 	defer t.mutex.Unlock()
 	conn, err := net.ListenUDP("udp", nil)
-	return conn, errors.Trace(err)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return conn, nil
 }
 
 func (t *testWebRTCDialCoordinator) UDPConn(_ context.Context, network, remoteAddress string) (net.PacketConn, error) {
@@ -338,7 +341,10 @@ func (t *testWebRTCDialCoordinator) UDPConn(_ context.Context, network, remoteAd
 		return nil, errors.TraceNew("invalid network")
 	}
 	conn, err := net.Dial(network, remoteAddress)
-	return conn.(*net.UDPConn), errors.Trace(err)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return conn.(*net.UDPConn), nil
 }
 
 func (t *testWebRTCDialCoordinator) BindToDevice(fileDescriptor int) error {

psiphon/common/protocol/packed_test.go

@@ -112,6 +112,8 @@ func makeTestPackValue(t *testing.T, spec packSpec) interface{} {
 		return []string{auth1, auth2}
 	case rawJSONConverter:
 		return []byte(fmt.Sprintf(`{"A":%d, "B":%d}`, prng.Intn(1>>32), prng.Intn(1>>32)))
+	case compatibleJSONMapConverter:
+		return []any{map[any]any{"a": 1, "b": 2}, map[any]any{"a": 3, "b": 4}}
 	}
 	t.Fatalf("unexpected converter")
 	return nil
@@ -125,6 +127,7 @@ func checkTestPackValues(
 
 	for name, spec := range specs {
 		originalValue := originalValues[name]
+		unpackedValue := unpackedValues[name]
 		if spec.converter == rawJSONConverter {
 
 			// Special case: for rawJSONConverter, the input is bytes while
@@ -132,10 +135,29 @@ func checkTestPackValues(
 			var unmarshaledJSON map[string]interface{}
 			_ = json.Unmarshal(originalValue.([]byte), &unmarshaledJSON)
 			originalValue = unmarshaledJSON
+
+		} else if spec.converter == compatibleJSONMapConverter {
+
+			// Special case: for compatibleJSONMapConverter, reverse the
+			// conversion to produce the original value with the same type.
+			unpackedSlice, ok := unpackedValue.([]map[string]interface{})
+			if !ok {
+				t.Errorf("expected []map[string]interface {} type")
+				return
+			}
+			entries := make([]interface{}, len(unpackedSlice))
+			for i, unpackedEntry := range unpackedSlice {
+				entry := make(map[interface{}]interface{})
+				for key, value := range unpackedEntry {
+					entry[key] = value
+				}
+				entries[i] = entry
+			}
+			unpackedValue = entries
 		}
-		if !reflect.DeepEqual(originalValue, unpackedValues[name]) {
+		if !reflect.DeepEqual(originalValue, unpackedValue) {
 			t.Errorf("decoded value %s not equal: %T %+v != %T %+v",
-				name, originalValue, originalValue, unpackedValues[name], unpackedValues[name])
+				name, originalValue, originalValue, unpackedValue, unpackedValue)
 		}
 	}
 }

psiphon/common/tactics/tactics_test.go

@@ -541,8 +541,19 @@ func TestTactics(t *testing.T) {
 		t.Fatalf("HandleTacticsPayload failed: %s", err)
 	}
 
-	if handshakeTacticsRecord == nil {
-		t.Fatalf("expected tactics record")
+	// When tactic parameters are unchanged, HandleTacticsPayload returns nil,
+	// so that callers do not apply tactics unnecessarily.
+	//
+	// Check that nil is returned, but then directly load the record stored by
+	// HandleTacticsPayload in order to check metadata including the updated
+	// TTL.
+
+	if handshakeTacticsRecord != nil {
+		t.Fatalf("unexpected tactics record")
+	}
+	handshakeTacticsRecord, err = getStoredTacticsRecord(storer, networkID)
+	if err != nil {
+		t.Fatalf("getStoredTacticsRecord failed: %s", err)
 	}
 
 	if fetchTacticsRecord.Tag != handshakeTacticsRecord.Tag {

psiphon/controller_test.go

@@ -382,7 +382,10 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 	// Note: a successful tactics request may modify config parameters.
 
 	var modifyConfig map[string]interface{}
-	json.Unmarshal(configJSON, &modifyConfig)
+	err = json.Unmarshal(configJSON, &modifyConfig)
+	if err != nil {
+		t.Fatalf("json.Unmarshal failed: %v", err)
+	}
 
 	modifyConfig["DataRootDirectory"] = testDataDirName
 

psiphon/memory_test/memory_test.go

@@ -96,7 +96,11 @@ func runMemoryTest(t *testing.T, testMode int) {
 	// Most of these fields _must_ be filled in before calling LoadConfig,
 	// so that they are correctly set into client parameters.
 	var modifyConfig map[string]interface{}
-	json.Unmarshal(configJSON, &modifyConfig)
+	err = json.Unmarshal(configJSON, &modifyConfig)
+	if err != nil {
+		t.Fatalf("json.Unmarshal failed: %v", err)
+	}
+
 	modifyConfig["ClientVersion"] = "999999999"
 	modifyConfig["TunnelPoolSize"] = 1
 	modifyConfig["DataRootDirectory"] = testDataDirName

psiphon/tactics_test.go

@@ -48,7 +48,10 @@ func TestStandAloneGetTactics(t *testing.T) {
 	}
 
 	var modifyConfig map[string]interface{}
-	json.Unmarshal(configJSON, &modifyConfig)
+	err = json.Unmarshal(configJSON, &modifyConfig)
+	if err != nil {
+		t.Fatalf("json.Unmarshal failed: %v", err)
+	}
 
 	modifyConfig["DataRootDirectory"] = testDataDirName