Apply review suggestions to the integration test

Author: MatteoPologruto

internal/integrationtest/arduino-cli.go

@@ -123,21 +123,14 @@ func NewArduinoCliWithinEnvironment(env *Environment, config *ArduinoCLIConfig)
 // It returns a testsuite.Environment and an ArduinoCLI client to perform the integration tests.
 // The Environment must be disposed by calling the CleanUp method via defer.
 func CreateEnvForDaemon(t *testing.T) (*Environment, *ArduinoCLI) {
-	return CreateEnvForDaemonWithUserAgent(t, "cli-test/0.0.0")
-}
-
-// CreateEnvForDaemonWithUserAgent performs the minimum required operations to start the arduino-cli daemon.
-// It returns a testsuite.Environment and an ArduinoCLI client to perform the integration tests.
-// The Environment must be disposed by calling the CleanUp method via defer.
-func CreateEnvForDaemonWithUserAgent(t *testing.T, userAgent string) (*Environment, *ArduinoCLI) {
 	env := NewEnvironment(t)
 
 	cli := NewArduinoCliWithinEnvironment(env, &ArduinoCLIConfig{
 		ArduinoCLIPath:         FindRepositoryRootPath(t).Join("arduino-cli"),
 		UseSharedStagingFolder: true,
 	})
 
-	_ = cli.StartDaemon(false, userAgent)
+	_ = cli.StartDaemon(false)
 	return env, cli
 }
 
@@ -417,7 +410,7 @@ func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader
 }
 
 // StartDaemon starts the Arduino CLI daemon. It returns the address of the daemon.
-func (cli *ArduinoCLI) StartDaemon(verbose bool, userAgent string) string {
+func (cli *ArduinoCLI) StartDaemon(verbose bool) string {
 	args := []string{"daemon", "--json"}
 	if cli.cliConfigPath != nil {
 		args = append([]string{"--config-file", cli.cliConfigPath.String()}, args...)
@@ -460,7 +453,7 @@ func (cli *ArduinoCLI) StartDaemon(verbose bool, userAgent string) string {
 		conn, err := grpc.NewClient(
 			cli.daemonAddr,
 			grpc.WithTransportCredentials(insecure.NewCredentials()),
-			grpc.WithUserAgent(userAgent),
+			grpc.WithUserAgent("cli-test/0.0.0"),
 		)
 		if err != nil {
 			connErr = err

internal/integrationtest/core/core_test.go

@@ -69,7 +69,7 @@ func TestCoreSearch(t *testing.T) {
 
 	// Set up an http server to serve our custom index file
 	test_index := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, test_index, false)
+	url := env.HTTPServeFile(8000, test_index)
 
 	// Run update-index with our test index
 	_, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String())
@@ -161,7 +161,7 @@ func TestCoreSearchNoArgs(t *testing.T) {
 
 	// Set up an http server to serve our custom index file
 	testIndex := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, testIndex, false)
+	url := env.HTTPServeFile(8000, testIndex)
 
 	// update custom index and install test core (installed cores affect `core search`)
 	_, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String())
@@ -749,7 +749,7 @@ func TestCoreSearchSortedResults(t *testing.T) {
 
 	// Set up the server to serve our custom index file
 	testIndex := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, testIndex, false)
+	url := env.HTTPServeFile(8000, testIndex)
 
 	// update custom index
 	_, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String())
@@ -821,7 +821,7 @@ func TestCoreListSortedResults(t *testing.T) {
 
 	// Set up the server to serve our custom index file
 	testIndex := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, testIndex, false)
+	url := env.HTTPServeFile(8000, testIndex)
 
 	// update custom index
 	_, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String())
@@ -892,7 +892,7 @@ func TestCoreListDeprecatedPlatformWithInstalledJson(t *testing.T) {
 
 	// Set up the server to serve our custom index file
 	testIndex := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, testIndex, false)
+	url := env.HTTPServeFile(8000, testIndex)
 
 	// update custom index
 	_, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String())
@@ -1110,8 +1110,8 @@ func TestCoreInstallRunsToolPostInstallScript(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()
 
-	url := env.HTTPServeFile(8080, paths.New("testdata", "package_with_postinstall_index.json"), false)
-	env.HTTPServeFile(8081, paths.New("testdata", "core_with_postinst.zip"), false)
+	url := env.HTTPServeFile(8080, paths.New("testdata", "package_with_postinstall_index.json"))
+	env.HTTPServeFile(8081, paths.New("testdata", "core_with_postinst.zip"))
 
 	_, _, err := cli.Run("core", "update-index", "--additional-urls", url.String())
 	require.NoError(t, err)
@@ -1129,7 +1129,7 @@ func TestCoreBrokenDependency(t *testing.T) {
 
 	// Set up an http server to serve our custom index file
 	test_index := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, test_index, false)
+	url := env.HTTPServeFile(8000, test_index)
 
 	// Run update-index with our test index
 	_, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String())
@@ -1145,7 +1145,7 @@ func TestCoreUpgradeWarningWithPackageInstalledButNotIndexed(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()
 
-	url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json"), false).String()
+	url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json")).String()
 
 	t.Run("missing additional-urls", func(t *testing.T) {
 		// update index
@@ -1187,7 +1187,7 @@ func TestCoreHavingIncompatibleDepTools(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()
 
-	url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json"), false).String()
+	url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json")).String()
 	additionalURLs := "--additional-urls=" + url
 
 	_, _, err := cli.Run("core", "update-index", additionalURLs)

internal/integrationtest/daemon/daemon_test.go

@@ -20,6 +20,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
+	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
 
@@ -562,15 +566,42 @@ func TestDaemonUserAgent(t *testing.T) {
 	// Set up an http server to serve our custom index file
 	// The user-agent is tested inside the HTTPServeFile function
 	test_index := paths.New("..", "testdata", "test_index.json")
-	url := env.HTTPServeFile(8000, test_index, true)
+	url := env.HTTPServeFile(8000, test_index)
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Test that the user-agent contains metadata from the context when the CLI is in daemon mode
+		userAgent := r.Header.Get("User-Agent")
+
+		require.Contains(t, userAgent, "cli-test/0.0.0")
+		require.Contains(t, userAgent, "grpc-go")
+		// Depends on how we built the client we may have git-snapshot or 0.0.0-git in dev releases
+		require.Condition(t, func() (success bool) {
+			return strings.Contains(userAgent, "arduino-cli/git-snapshot") ||
+				strings.Contains(userAgent, "arduino-cli/0.0.0-git")
+		})
+
+		proxiedReq, err := http.NewRequest(r.Method, url.String(), r.Body)
+		require.NoError(t, err)
+		maps.Copy(proxiedReq.Header, r.Header)
+
+		proxiedResp, err := http.DefaultTransport.RoundTrip(proxiedReq)
+		require.NoError(t, err)
+		defer proxiedResp.Body.Close()
+
+		// Copy the headers from the proxy response to the original response
+		maps.Copy(r.Header, proxiedReq.Header)
+		w.WriteHeader(proxiedResp.StatusCode)
+		io.Copy(w, proxiedResp.Body)
+	}))
+	defer ts.Close()
 
 	grpcInst := cli.Create()
 	require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
 		fmt.Printf("INIT> %v\n", ir.GetMessage())
 	}))
 
 	// Set extra indexes
-	err := cli.SetValue("board_manager.additional_urls", `["http://127.0.0.1:8000/test_index.json"]`)
+	additionalURL := ts.URL + "/test_index.json"
+	err := cli.SetValue("board_manager.additional_urls", fmt.Sprintf(`["%s"]`, additionalURL))
 	require.NoError(t, err)
 
 	{
@@ -579,7 +610,7 @@ func TestDaemonUserAgent(t *testing.T) {
 		res, err := analyzeUpdateIndexClient(t, cl)
 		require.NoError(t, err)
 		require.Len(t, res, 2)
-		require.True(t, res[url.String()].GetSuccess())
+		require.True(t, res[additionalURL].GetSuccess())
 	}
 }
 

internal/integrationtest/http_server.go

@@ -26,18 +26,11 @@ import (
 
 // HTTPServeFile spawn an http server that serve a single file. The server
 // is started on the given port. The URL to the file and a cleanup function are returned.
-func (env *Environment) HTTPServeFile(port uint16, path *paths.Path, isDaemon bool) *url.URL {
+func (env *Environment) HTTPServeFile(port uint16, path *paths.Path) *url.URL {
 	t := env.T()
 	mux := http.NewServeMux()
 	mux.HandleFunc("/"+path.Base(), func(w http.ResponseWriter, r *http.Request) {
 		http.ServeFile(w, r, path.String())
-		if isDaemon {
-			// Test that the user-agent contains metadata from the context when the CLI is in daemon mode
-			userAgent := r.Header.Get("User-Agent")
-			require.Contains(t, userAgent, "arduino-cli/git-snapshot")
-			require.Contains(t, userAgent, "cli-test/0.0.0")
-			require.Contains(t, userAgent, "grpc-go")
-		}
 	})
 	server := &http.Server{
 		Addr:    fmt.Sprintf(":%d", port),