Fix: can't set test parameters via SetClientParameters

- When actual tactics payload is applied, parameters are
  reset, config parameters are applied, and then new tactics
  are applied.

- In this process, the temporary settings applied using
  SetClientParameters are lost.

psiphon/controller_test.go

@@ -452,6 +452,22 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 	modifyConfig["DataStoreDirectory"] = testDataDirName
 	modifyConfig["RemoteServerListDownloadFilename"] = filepath.Join(testDataDirName, "server_list_compressed")
 	modifyConfig["UpgradeDownloadFilename"] = filepath.Join(testDataDirName, "upgrade")
+
+	if runConfig.protocol != "" {
+		modifyConfig["TunnelProtocols"] = protocol.TunnelProtocols{runConfig.protocol}
+	}
+
+	// Override client retry throttle values to speed up automated
+	// tests and ensure tests complete within fixed deadlines.
+	modifyConfig["FetchRemoteServerListRetryPeriodMilliseconds"] = 250
+	modifyConfig["FetchUpgradeRetryPeriodMilliseconds"] = 250
+	modifyConfig["EstablishTunnelPausePeriodSeconds"] = 1
+
+	if runConfig.disableUntunneledUpgrade {
+		// Disable untunneled upgrade downloader to ensure tunneled case is tested
+		modifyConfig["UpgradeDownloadClientVersionHeader"] = "invalid-value"
+	}
+
 	configJSON, _ = json.Marshal(modifyConfig)
 
 	config, err := LoadConfig(configJSON)
@@ -492,31 +508,17 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 	// that the tactics request succeeds.
 	config.NetworkIDGetter = &testNetworkGetter{}
 
-	// The following config values must be applied through client parameters
-	// (setting the fields in Config directly will have no effect since the
-	// client parameters have been populated by LoadConfig).
+	// The following values can only be applied through client parameters.
+	// TODO: a successful tactics request can reset these parameters.
 
 	applyParameters := make(map[string]interface{})
 
-	if runConfig.disableUntunneledUpgrade {
-		// Disable untunneled upgrade downloader to ensure tunneled case is tested
-		applyParameters[parameters.UpgradeDownloadClientVersionHeader] = ""
-	}
-
 	if runConfig.transformHostNames {
 		applyParameters[parameters.TransformHostNameProbability] = 1.0
 	} else {
 		applyParameters[parameters.TransformHostNameProbability] = 0.0
 	}
 
-	// Override client retry throttle values to speed up automated
-	// tests and ensure tests complete within fixed deadlines.
-	applyParameters[parameters.FetchRemoteServerListRetryPeriod] = "250ms"
-	applyParameters[parameters.FetchUpgradeRetryPeriod] = "250ms"
-	applyParameters[parameters.EstablishTunnelPausePeriod] = "250ms"
-
-	applyParameters[parameters.LimitTunnelProtocols] = protocol.TunnelProtocols{runConfig.protocol}
-
 	err = config.SetClientParameters("", true, applyParameters)
 	if err != nil {
 		t.Fatalf("SetClientParameters failed: %s", err)
@@ -580,6 +582,7 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 			case "ConnectingServer":
 
 				serverProtocol := payload["protocol"].(string)
+
 				if runConfig.protocol != "" && serverProtocol != runConfig.protocol {
 					// TODO: wrong goroutine for t.FatalNow()
 					t.Fatalf("wrong protocol selected: %s", serverProtocol)

psiphon/server/server_test.go

@@ -542,15 +542,18 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 	localHTTPProxyPort := 8081
 
 	// Note: calling LoadConfig ensures the Config is fully initialized
-	clientConfigJSON := `
+	clientConfigJSON := fmt.Sprintf(`
     {
         "ClientPlatform" : "Windows",
         "ClientVersion" : "0",
         "SponsorId" : "0",
         "PropagationChannelId" : "0",
         "DisableRemoteServerListFetcher" : true,
-        "UseIndistinguishableTLS" : true
-    }`
+        "UseIndistinguishableTLS" : true,
+        "EstablishTunnelPausePeriodSeconds" : 1,
+        "ConnectionWorkerPoolSize" : %d,
+        "TunnelProtocols" : ["%s"]
+    }`, numTunnels, runConfig.tunnelProtocol)
 	clientConfig, _ := psiphon.LoadConfig([]byte(clientConfigJSON))
 
 	clientConfig.DataStoreDirectory = testDataDirName
@@ -582,27 +585,18 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 		clientConfig.NetworkIDGetter = &testNetworkGetter{}
 	}
 
-	// The following config values must be applied through client parameters
-	// (setting the fields in Config directly will have no effect since the
-	// client parameters have been populated by LoadConfig).
-
-	applyParameters := make(map[string]interface{})
-
-	applyParameters[parameters.ConnectionWorkerPoolSize] = numTunnels
-
-	applyParameters[parameters.EstablishTunnelPausePeriod] = "250ms"
-
-	applyParameters[parameters.LimitTunnelProtocols] = protocol.TunnelProtocols{runConfig.tunnelProtocol}
-
 	if doTactics {
 		// Configure nonfunctional values that must be overridden by tactics.
+
+		applyParameters := make(map[string]interface{})
+
 		applyParameters[parameters.TunnelConnectTimeout] = "1s"
 		applyParameters[parameters.TunnelRateLimits] = common.RateLimits{WriteBytesPerSecond: 1}
-	}
 
-	err = clientConfig.SetClientParameters("", true, applyParameters)
-	if err != nil {
-		t.Fatalf("SetClientParameters failed: %s", err)
+		err = clientConfig.SetClientParameters("", true, applyParameters)
+		if err != nil {
+			t.Fatalf("SetClientParameters failed: %s", err)
+		}
 	}
 
 	controller, err := psiphon.NewController(clientConfig)