[skip-changelog] Some integration test refactorings (#1911)

* Integrated 'go.bug.st/testsuite'

It doesn't make much sense to keep it separate in a different library

* Do not use shared download dir for 'cache clean' command tests

* Added method to get cli download dir

* Create infra for sharing directories between tests

* Slighlty improved test output

* Non-parallel testing is no longer required

* Add more time to complete tests

* Update internal/integrationtest/cache/cache_test.go
Co-authored-by: MatteoPologruto <109663225+MatteoPologruto@users.noreply.github.com>
Co-authored-by: MatteoPologruto <109663225+MatteoPologruto@users.noreply.github.com>

Taskfile.yml

@@ -95,14 +95,12 @@ tasks:
       - task: go:build
     dir: '{{default "./" .GO_MODULE_PATH}}'
     cmds:
-      # "-p 1" will not run test in parallel, this is required for integration tests
       - |
         go test \
           -v \
           -short \
-          -p 1 \
           -run '{{default ".*" .GO_TEST_REGEX}}' \
-          {{default "-timeout 15m -coverpkg=./... -covermode=atomic" .GO_TEST_FLAGS}} \
+          {{default "-timeout 20m -coverpkg=./... -covermode=atomic" .GO_TEST_FLAGS}} \
           -coverprofile=coverage_unit.txt \
           {{default .DEFAULT_GO_PACKAGES .GO_PACKAGES}} \
           {{.TEST_LDFLAGS}}

arduino/discovery/discovery_client/go.sum

@@ -315,7 +315,6 @@ go.bug.st/relaxed-semver v0.9.0/go.mod h1:ug0/W/RPYUjliE70Ghxg77RDHmPxqpo7SHV16i
 go.bug.st/serial v1.3.2/go.mod h1:jDkjqASf/qSjmaOxHSHljwUQ6eHo/ZX/bxJLQqSlvZg=
 go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45/go.mod h1:dRSl/CVCTf56CkXgJMDOdSwNfo2g1orOGE/gBGdvjZw=
 go.bug.st/testifyjson v1.1.1/go.mod h1:nZyy2icFbv3OE3zW3mGVOnC/GhWgb93LRu+29n2tJlI=
-go.bug.st/testsuite v0.1.0/go.mod h1:xCIDf97kf9USoz960Foy3CoquwhQmfuFRNh9git70as=
 go.etcd.io/etcd/api/v3 v3.5.0/go.mod h1:cbVKeC6lCfl7j/8jBhAK6aIYO9XOjdptoxU/nLQcPvs=
 go.etcd.io/etcd/client/pkg/v3 v3.5.0/go.mod h1:IJHfcCEKxYu1Os13ZdwCwIUTUVGYTSAM3YSwc9/Ac1g=
 go.etcd.io/etcd/client/v2 v2.305.0/go.mod h1:h9puh54ZTgAKtEbut2oe9P4L/oqKCVB6xsXlzd7alYQ=

client_example/go.sum

@@ -297,7 +297,6 @@ go.bug.st/relaxed-semver v0.9.0/go.mod h1:ug0/W/RPYUjliE70Ghxg77RDHmPxqpo7SHV16i
 go.bug.st/serial v1.3.2/go.mod h1:jDkjqASf/qSjmaOxHSHljwUQ6eHo/ZX/bxJLQqSlvZg=
 go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45/go.mod h1:dRSl/CVCTf56CkXgJMDOdSwNfo2g1orOGE/gBGdvjZw=
 go.bug.st/testifyjson v1.1.1/go.mod h1:nZyy2icFbv3OE3zW3mGVOnC/GhWgb93LRu+29n2tJlI=
-go.bug.st/testsuite v0.1.0/go.mod h1:xCIDf97kf9USoz960Foy3CoquwhQmfuFRNh9git70as=
 go.etcd.io/etcd/api/v3 v3.5.0/go.mod h1:cbVKeC6lCfl7j/8jBhAK6aIYO9XOjdptoxU/nLQcPvs=
 go.etcd.io/etcd/client/pkg/v3 v3.5.0/go.mod h1:IJHfcCEKxYu1Os13ZdwCwIUTUVGYTSAM3YSwc9/Ac1g=
 go.etcd.io/etcd/client/v2 v2.305.0/go.mod h1:h9puh54ZTgAKtEbut2oe9P4L/oqKCVB6xsXlzd7alYQ=

docsgen/go.sum

@@ -373,7 +373,6 @@ go.bug.st/serial v1.3.2/go.mod h1:jDkjqASf/qSjmaOxHSHljwUQ6eHo/ZX/bxJLQqSlvZg=
 go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45 h1:mACY1anK6HNCZtm/DK2Rf2ZPHggVqeB0+7rY9Gl6wyI=
 go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45/go.mod h1:dRSl/CVCTf56CkXgJMDOdSwNfo2g1orOGE/gBGdvjZw=
 go.bug.st/testifyjson v1.1.1/go.mod h1:nZyy2icFbv3OE3zW3mGVOnC/GhWgb93LRu+29n2tJlI=
-go.bug.st/testsuite v0.1.0/go.mod h1:xCIDf97kf9USoz960Foy3CoquwhQmfuFRNh9git70as=
 go.etcd.io/etcd/api/v3 v3.5.0/go.mod h1:cbVKeC6lCfl7j/8jBhAK6aIYO9XOjdptoxU/nLQcPvs=
 go.etcd.io/etcd/client/pkg/v3 v3.5.0/go.mod h1:IJHfcCEKxYu1Os13ZdwCwIUTUVGYTSAM3YSwc9/Ac1g=
 go.etcd.io/etcd/client/v2 v2.305.0/go.mod h1:h9puh54ZTgAKtEbut2oe9P4L/oqKCVB6xsXlzd7alYQ=

go.mod

@@ -58,8 +58,8 @@ require (
 )
 
 require (
+	github.com/rogpeppe/go-internal v1.3.0
 	go.bug.st/testifyjson v1.1.1
-	go.bug.st/testsuite v0.1.0
 )
 
 require (

go.sum

@@ -310,6 +310,7 @@ github.com/rifflock/lfshook v0.0.0-20180920164130-b9218ef580f5/go.mod h1:GEXHk5H
 github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY=
 github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
 github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
+github.com/rogpeppe/go-internal v1.3.0 h1:RR9dF3JtopPvtkroDZuVD7qquD0bnHlKSqaQhgwt8yk=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
@@ -376,8 +377,6 @@ go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45 h1:mACY1anK6HNCZtm/DK2Rf2
 go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45/go.mod h1:dRSl/CVCTf56CkXgJMDOdSwNfo2g1orOGE/gBGdvjZw=
 go.bug.st/testifyjson v1.1.1 h1:nHotIMK151LF3vYsU/b2RaoVaWCgrf2kvQeGNoZkGaA=
 go.bug.st/testifyjson v1.1.1/go.mod h1:nZyy2icFbv3OE3zW3mGVOnC/GhWgb93LRu+29n2tJlI=
-go.bug.st/testsuite v0.1.0 h1:oX4zdIB62+G5A0Kq4dja7Vy8tDiKqKVhhxkzhpMGgog=
-go.bug.st/testsuite v0.1.0/go.mod h1:xCIDf97kf9USoz960Foy3CoquwhQmfuFRNh9git70as=
 go.etcd.io/etcd/api/v3 v3.5.0/go.mod h1:cbVKeC6lCfl7j/8jBhAK6aIYO9XOjdptoxU/nLQcPvs=
 go.etcd.io/etcd/client/pkg/v3 v3.5.0/go.mod h1:IJHfcCEKxYu1Os13ZdwCwIUTUVGYTSAM3YSwc9/Ac1g=
 go.etcd.io/etcd/client/v2 v2.305.0/go.mod h1:h9puh54ZTgAKtEbut2oe9P4L/oqKCVB6xsXlzd7alYQ=

internal/integrationtest/arduino-cli.go

@@ -33,14 +33,9 @@ import (
 	"github.com/arduino/go-paths-helper"
 	"github.com/fatih/color"
 	"github.com/stretchr/testify/require"
-	"go.bug.st/testsuite"
 	"google.golang.org/grpc"
 )
 
-func init() {
-	testsuite.ProjectName = "cli"
-}
-
 // FindRepositoryRootPath returns the repository root path
 func FindRepositoryRootPath(t *testing.T) *paths.Path {
 	repoRootPath, err := paths.Getwd()
@@ -52,13 +47,18 @@ func FindRepositoryRootPath(t *testing.T) *paths.Path {
 	return repoRootPath
 }
 
+// FindArduinoCLIPath returns the path to the arduino-cli executable
+func FindArduinoCLIPath(t *testing.T) *paths.Path {
+	return FindRepositoryRootPath(t).Join("arduino-cli")
+}
+
 // CreateArduinoCLIWithEnvironment performs the minimum amount of actions
 // to build the default test environment.
-func CreateArduinoCLIWithEnvironment(t *testing.T) (*testsuite.Environment, *ArduinoCLI) {
-	env := testsuite.NewEnvironment(t)
+func CreateArduinoCLIWithEnvironment(t *testing.T) (*Environment, *ArduinoCLI) {
+	env := NewEnvironment(t)
 
 	cli := NewArduinoCliWithinEnvironment(env, &ArduinoCLIConfig{
-		ArduinoCLIPath:         FindRepositoryRootPath(t).Join("arduino-cli"),
+		ArduinoCLIPath:         FindArduinoCLIPath(t),
 		UseSharedStagingFolder: true,
 	})
 
@@ -89,7 +89,7 @@ type ArduinoCLIConfig struct {
 }
 
 // NewArduinoCliWithinEnvironment creates a new Arduino CLI client inside the given environment.
-func NewArduinoCliWithinEnvironment(env *testsuite.Environment, config *ArduinoCLIConfig) *ArduinoCLI {
+func NewArduinoCliWithinEnvironment(env *Environment, config *ArduinoCLIConfig) *ArduinoCLI {
 	color.NoColor = false
 	cli := &ArduinoCLI{
 		path:          config.ArduinoCLIPath,
@@ -100,7 +100,11 @@ func NewArduinoCliWithinEnvironment(env *testsuite.Environment, config *ArduinoC
 		workingDir:    env.RootDir(),
 	}
 	if config.UseSharedStagingFolder {
-		cli.stagingDir = env.SharedDownloadsDir()
+		sharedDir := env.SharedDownloadsDir()
+		cli.stagingDir = sharedDir.Lock()
+		env.RegisterCleanUpCallback(func() {
+			sharedDir.Unlock()
+		})
 	}
 
 	cli.cliEnvVars = map[string]string{
@@ -137,6 +141,11 @@ func (cli *ArduinoCLI) WorkingDir() *paths.Path {
 	return cli.workingDir
 }
 
+// DownloadDir returns the download directory
+func (cli *ArduinoCLI) DownloadDir() *paths.Path {
+	return cli.stagingDir
+}
+
 // CopySketch copies a sketch inside the testing environment and returns its path
 func (cli *ArduinoCLI) CopySketch(sketchName string) *paths.Path {
 	p, err := paths.Getwd()

internal/integrationtest/cache/cache_test.go

@@ -23,8 +23,11 @@ import (
 )
 
 func TestCacheClean(t *testing.T) {
-	// Clean the cache under arduino caching file directory which is "<Arduino configure file path>/staging"
-	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	// This test should not use shared download directory because it will be cleaned up with 'cache clean' command
+	env := integrationtest.NewEnvironment(t)
+	cli := integrationtest.NewArduinoCliWithinEnvironment(env, &integrationtest.ArduinoCLIConfig{
+		ArduinoCLIPath: integrationtest.FindArduinoCLIPath(t),
+	})
 	defer env.CleanUp()
 
 	_, _, err := cli.Run("cache", "clean")

internal/integrationtest/config/config_test.go

@@ -50,7 +50,7 @@ func TestInitWithExistingCustomConfig(t *testing.T) {
 	require.Equal(t, config["board_manager"]["additional_urls"].([]interface{})[0].(string), "https://example.com")
 	require.Equal(t, config["daemon"]["port"].(string), "50051")
 	require.Equal(t, config["directories"]["data"].(string), cli.DataDir().String())
-	require.Equal(t, config["directories"]["downloads"].(string), env.SharedDownloadsDir().String())
+	require.Equal(t, config["directories"]["downloads"].(string), cli.DownloadDir().String())
 	require.Equal(t, config["directories"]["user"].(string), cli.SketchbookDir().String())
 	require.Empty(t, config["logging"]["file"])
 	require.Equal(t, config["logging"]["format"].(string), "text")
@@ -71,7 +71,7 @@ func TestInitWithExistingCustomConfig(t *testing.T) {
 	require.Empty(t, config["board_manager"]["additional_urls"])
 	require.Equal(t, config["daemon"]["port"].(string), "50051")
 	require.Equal(t, config["directories"]["data"].(string), cli.DataDir().String())
-	require.Equal(t, config["directories"]["downloads"].(string), env.SharedDownloadsDir().String())
+	require.Equal(t, config["directories"]["downloads"].(string), cli.DownloadDir().String())
 	require.Equal(t, config["directories"]["user"].(string), cli.SketchbookDir().String())
 	require.Empty(t, config["logging"]["file"])
 	require.Equal(t, config["logging"]["format"].(string), "text")
@@ -96,7 +96,7 @@ func TestInitOverwriteExistingCustomFile(t *testing.T) {
 	require.Equal(t, config["board_manager"]["additional_urls"].([]interface{})[0].(string), "https://example.com")
 	require.Equal(t, config["daemon"]["port"].(string), "50051")
 	require.Equal(t, config["directories"]["data"].(string), cli.DataDir().String())
-	require.Equal(t, config["directories"]["downloads"].(string), env.SharedDownloadsDir().String())
+	require.Equal(t, config["directories"]["downloads"].(string), cli.DownloadDir().String())
 	require.Equal(t, config["directories"]["user"].(string), cli.SketchbookDir().String())
 	require.Empty(t, config["logging"]["file"])
 	require.Equal(t, config["logging"]["format"].(string), "text")
@@ -115,7 +115,7 @@ func TestInitOverwriteExistingCustomFile(t *testing.T) {
 	require.Empty(t, config["board_manager"]["additional_urls"])
 	require.Equal(t, config["daemon"]["port"].(string), "50051")
 	require.Equal(t, config["directories"]["data"].(string), cli.DataDir().String())
-	require.Equal(t, config["directories"]["downloads"].(string), env.SharedDownloadsDir().String())
+	require.Equal(t, config["directories"]["downloads"].(string), cli.DownloadDir().String())
 	require.Equal(t, config["directories"]["user"].(string), cli.SketchbookDir().String())
 	require.Empty(t, config["logging"]["file"])
 	require.Equal(t, config["logging"]["format"].(string), "text")

internal/integrationtest/daemon/daemon_test.go

@@ -20,14 +20,13 @@ import (
 
 	"github.com/arduino/arduino-cli/internal/integrationtest"
 	"github.com/stretchr/testify/require"
-	"go.bug.st/testsuite"
 )
 
 // createEnvForDaemon 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 createEnvForDaemon(t *testing.T) (*testsuite.Environment, *integrationtest.ArduinoCLI) {
-	env := testsuite.NewEnvironment(t)
+func createEnvForDaemon(t *testing.T) (*integrationtest.Environment, *integrationtest.ArduinoCLI) {
+	env := integrationtest.NewEnvironment(t)
 
 	cli := integrationtest.NewArduinoCliWithinEnvironment(env, &integrationtest.ArduinoCLIConfig{
 		ArduinoCLIPath:         integrationtest.FindRepositoryRootPath(t).Join("arduino-cli"),

internal/integrationtest/environment.go

+// This file is part of arduino-cli.
+//
+// Copyright 2022 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package integrationtest
+
+import (
+	"testing"
+
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+)
+
+// ProjectName is the prefix used in the test temp files
+var ProjectName = "cli"
+
+// Environment is a test environment for the test suite.
+type Environment struct {
+	rootDir      *paths.Path
+	downloadsDir *SharedDir
+	t            *testing.T
+	cleanUp      func()
+}
+
+// NewEnvironment creates a new test environment.
+func NewEnvironment(t *testing.T) *Environment {
+	downloadsDir := NewSharedDir(t, "downloads")
+	rootDir, err := paths.MkTempDir("", ProjectName)
+	require.NoError(t, err)
+	return &Environment{
+		rootDir:      rootDir,
+		downloadsDir: downloadsDir,
+		t:            t,
+		cleanUp: func() {
+			require.NoError(t, rootDir.RemoveAll())
+		},
+	}
+}
+
+// RegisterCleanUpCallback adds a clean up function to the clean up chain
+func (e *Environment) RegisterCleanUpCallback(newCleanUp func()) {
+	previousCleanUp := e.cleanUp
+	e.cleanUp = func() {
+		newCleanUp()
+		previousCleanUp()
+	}
+}
+
+// CleanUp removes the test environment.
+func (e *Environment) CleanUp() {
+	e.cleanUp()
+}
+
+// RootDir returns the root dir of the environment.
+func (e *Environment) RootDir() *paths.Path {
+	return e.rootDir
+}
+
+// SharedDownloadsDir return the shared directory for downloads
+func (e *Environment) SharedDownloadsDir() *SharedDir {
+	return e.downloadsDir
+}
+
+// T returns the testing environment
+func (e *Environment) T() *testing.T {
+	return e.t
+}

internal/integrationtest/http_server.go

+// This file is part of arduino-cli.
+//
+// Copyright 2022 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package integrationtest
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+)
+
+// 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) *url.URL {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/"+path.Base(), func(w http.ResponseWriter, r *http.Request) {
+		http.ServeFile(w, r, path.String())
+	})
+	server := &http.Server{
+		Addr:    fmt.Sprintf(":%d", port),
+		Handler: mux,
+	}
+
+	t := env.T()
+	fileURL, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d/%s", port, path.Base()))
+	require.NoError(t, err)
+
+	go func() {
+		err := server.ListenAndServe()
+		require.Equal(t, err, http.ErrServerClosed)
+	}()
+
+	env.RegisterCleanUpCallback(func() {
+		server.Close()
+	})
+
+	return fileURL
+}

internal/integrationtest/main/main_test.go

@@ -46,7 +46,7 @@ func TestVersion(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, string(stdout), "Version:")
 	require.Contains(t, string(stdout), "Commit:")
-	require.Empty(t, stderr)
+	require.Empty(t, string(stderr))
 
 	// Checks if "version --format json" has a json as an output
 	stdout, _, err = cli.Run("version", "--format", "json")

internal/integrationtest/shared_directory.go

+// This file is part of arduino-cli.
+//
+// Copyright 2022 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package integrationtest
+
+import (
+	"sync"
+	"testing"
+
+	"github.com/arduino/go-paths-helper"
+	"github.com/rogpeppe/go-internal/lockedfile"
+	"github.com/stretchr/testify/require"
+)
+
+// SharedDir is a directory that is shared between multiple tests.
+type SharedDir struct {
+	dir      *paths.Path
+	t        *testing.T
+	mux      sync.Mutex
+	fileLock *lockedfile.File
+}
+
+// Lock locks the shared directory for exclusive access and return the path to the directory.
+func (d *SharedDir) Lock() *paths.Path {
+	d.mux.Lock()
+	defer d.mux.Unlock()
+	if d.fileLock != nil {
+		panic("SharedDir already locked")
+	}
+	fileLock, err := lockedfile.Create(d.dir.Join(".lock").String())
+	require.NoError(d.t, err)
+	d.fileLock = fileLock
+	return d.dir
+}
+
+// Unlock unlocks the shared directory.
+func (d *SharedDir) Unlock() {
+	d.mux.Lock()
+	defer d.mux.Unlock()
+	if d.fileLock == nil {
+		panic("SharedDir already unlocked")
+	}
+	require.NoError(d.t, d.fileLock.Close())
+	d.fileLock = nil
+}
+
+// NewSharedDir creates a new shared directory.
+func NewSharedDir(t *testing.T, id string) *SharedDir {
+	dir := paths.TempDir().Join(ProjectName + "-" + id)
+	require.NoError(t, dir.MkdirAll())
+	return &SharedDir{
+		dir: dir,
+		t:   t,
+	}
+}