From ecc1ece83b0d39588496a16b093e7bc6ad057043 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Wed, 29 Nov 2023 15:10:29 +0100
Subject: [PATCH] [skip-changelog] Parallelize `upload_mock` integration tests
 (#2444)

* Parallelize upload_mock integration tests

* Accumulate output in integration test' runs of arduino-cli

Otherwise the output may be interleaved if tests are run in parallel.
---
 internal/integrationtest/arduino-cli.go       | 25 ++++++++++++-------
 .../upload_mock/upload_mock_test.go           | 20 ++++++++-------
 2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/internal/integrationtest/arduino-cli.go b/internal/integrationtest/arduino-cli.go
index 2f1f7c57842..34615016f29 100644
--- a/internal/integrationtest/arduino-cli.go
+++ b/internal/integrationtest/arduino-cli.go
@@ -299,13 +299,20 @@ func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader
 		args = append([]string{"--config-file", cli.cliConfigPath.String()}, args...)
 	}
 
+	// Accumulate all output to terminal and spit-out all at once at the end of the test
+	// This allows to correctly group test output when running t.Parallel() tests.
+	terminalOut := new(bytes.Buffer)
+	defer func() {
+		fmt.Print(terminalOut.String())
+	}()
+
 	// Github-actions workflow tags to fold log lines
 	if os.Getenv("GITHUB_ACTIONS") != "" {
-		fmt.Printf("::group::Running %s\n", strings.Join(args, " "))
-		defer fmt.Println("::endgroup::")
+		fmt.Fprintf(terminalOut, "::group::Running %s\n", strings.Join(args, " "))
+		defer fmt.Fprintln(terminalOut, "::endgroup::")
 	}
 
-	fmt.Println(color.HiBlackString(">>> Running: ") + color.HiYellowString("%s %s", cli.path, strings.Join(args, " ")))
+	fmt.Fprintln(terminalOut, color.HiBlackString(">>> Running: ")+color.HiYellowString("%s %s", cli.path, strings.Join(args, " ")))
 	cliProc, err := executils.NewProcessFromPath(cli.convertEnvForExecutils(env), cli.path, args...)
 	cli.t.NoError(err)
 	stdout, err := cliProc.StdoutPipe()
@@ -325,8 +332,8 @@ func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader
 		if stdoutBuff == nil {
 			stdoutBuff = io.Discard
 		}
-		if _, err := io.Copy(stdoutBuff, io.TeeReader(stdout, os.Stdout)); err != nil {
-			fmt.Println(color.HiBlackString("<<< stdout copy error:"), err)
+		if _, err := io.Copy(stdoutBuff, io.TeeReader(stdout, terminalOut)); err != nil {
+			fmt.Fprintln(terminalOut, color.HiBlackString("<<< stdout copy error:"), err)
 		}
 	}()
 	go func() {
@@ -334,20 +341,20 @@ func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader
 		if stderrBuff == nil {
 			stderrBuff = io.Discard
 		}
-		if _, err := io.Copy(stderrBuff, io.TeeReader(stderr, os.Stderr)); err != nil {
-			fmt.Println(color.HiBlackString("<<< stderr copy error:"), err)
+		if _, err := io.Copy(stderrBuff, io.TeeReader(stderr, terminalOut)); err != nil {
+			fmt.Fprintln(terminalOut, color.HiBlackString("<<< stderr copy error:"), err)
 		}
 	}()
 	if stdinBuff != nil {
 		go func() {
 			if _, err := io.Copy(stdin, stdinBuff); err != nil {
-				fmt.Println(color.HiBlackString("<<< stdin copy error:"), err)
+				fmt.Fprintln(terminalOut, color.HiBlackString("<<< stdin copy error:"), err)
 			}
 		}()
 	}
 	wg.Wait()
 	cliErr := cliProc.Wait()
-	fmt.Println(color.HiBlackString("<<< Run completed (err = %v)", cliErr))
+	fmt.Fprintln(terminalOut, color.HiBlackString("<<< Run completed (err = %v)", cliErr))
 
 	return cliErr
 }
diff --git a/internal/integrationtest/upload_mock/upload_mock_test.go b/internal/integrationtest/upload_mock/upload_mock_test.go
index d88fa84a91d..c41f9db2f7d 100644
--- a/internal/integrationtest/upload_mock/upload_mock_test.go
+++ b/internal/integrationtest/upload_mock/upload_mock_test.go
@@ -44,7 +44,7 @@ type parametersMap struct {
 
 func TestUploadSketch(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
-	defer env.CleanUp()
+	t.Cleanup(env.CleanUp)
 
 	indexes := []string{
 		"https://github.com/stm32duino/BoardManagerFiles/raw/main/package_stmicroelectronics_index.json",
@@ -655,13 +655,14 @@ func TestUploadSketch(t *testing.T) {
 	sketchPath := cli.SketchbookDir().Join(sketchName)
 	_, _, err := cli.Run("sketch", "new", sketchPath.String())
 	require.NoError(t, err)
+	buildDir := generateBuildDir(sketchPath, t)
+	t.Cleanup(func() { buildDir.RemoveAll() })
 
-	var stdout []byte
-
-	for i, test := range testParameters {
+	for i, _test := range testParameters {
+		test := _test
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
-			buildDir := generateBuildDir(sketchPath, t)
-			defer buildDir.RemoveAll()
+			t.Parallel()
+			var stdout []byte
 			if test.Programmer != "" {
 				if test.UploadPort != "" {
 					stdout, _, err = cli.Run("upload", "-p", test.UploadPort, "-P", test.Programmer, "-b", test.Fqbn, sketchPath.String(), "--dry-run", "-v")
@@ -686,10 +687,11 @@ func TestUploadSketch(t *testing.T) {
 		})
 	}
 
-	for i, test := range testParametersMap {
+	for i, _test := range testParametersMap {
+		test := _test
 		t.Run(fmt.Sprintf("WithMap%d", i), func(t *testing.T) {
-			buildDir := generateBuildDir(sketchPath, t)
-			defer buildDir.RemoveAll()
+			t.Parallel()
+			var stdout []byte
 			if test.Programmer != "" {
 				if test.UploadPort != "" {
 					stdout, _, err = cli.Run("upload", "-p", test.UploadPort, "-P", test.Programmer, "-b", test.Fqbn, sketchPath.String(), "--dry-run", "-v")