Move output dir logic to suite (#29580)

test/new-e2e/pkg/e2e/suite.go

@@ -146,6 +146,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"sync"
 	"testing"
 	"time"
 
@@ -189,6 +190,9 @@ type BaseSuite[Env any] struct {
 	currentProvisioners  ProvisionerMap
 
 	firstFailTest string
+
+	testSessionOutputDir     string
+	onceTestSessionOutputDir sync.Once
 }
 
 //
@@ -529,6 +533,32 @@ func (bs *BaseSuite[Env]) TearDownSuite() {
 	}
 }
 
+// GetRootOutputDir returns the root output directory for tests to store output files and artifacts.
+// The directory is created on the first call to this function and reused in future calls.
+//
+// See BaseSuite.CreateTestOutputDir() for a function that returns a directory for the current test.
+//
+// See CreateRootOutputDir() for details on the root directory creation.
+func (bs *BaseSuite[Env]) GetRootOutputDir() (string, error) {
+	var err error
+	bs.onceTestSessionOutputDir.Do(func() {
+		// Store the timestamped directory to be used by all tests in the suite
+		bs.testSessionOutputDir, err = CreateRootOutputDir()
+	})
+	return bs.testSessionOutputDir, err
+}
+
+// CreateTestOutputDir returns an output directory for the current test.
+//
+// See also CreateTestOutputDir()
+func (bs *BaseSuite[Env]) CreateTestOutputDir() (string, error) {
+	root, err := bs.GetRootOutputDir()
+	if err != nil {
+		return "", err
+	}
+	return CreateTestOutputDir(root, bs.T())
+}
+
 // Run is a helper function to run a test suite.
 // Unfortunately, we cannot use `s Suite[Env]` as Go is not able to match it with a struct
 // However it's able to verify the same constraint on T

test/new-e2e/pkg/e2e/suite_utils.go

@@ -5,7 +5,17 @@
 
 package e2e
 
-import "testing"
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner"
+
+	"testing"
+)
 
 type testLogger struct {
 	t *testing.T
@@ -20,3 +30,73 @@ func (tl testLogger) Write(p []byte) (n int, err error) {
 	tl.t.Log(string(p))
 	return len(p), nil
 }
+
+// CreateRootOutputDir creates and returns a directory for tests to store output files and artifacts.
+// A timestamp is included in the path to distinguish between multiple runs, and os.MkdirTemp() is
+// used to avoid name collisions between parallel runs.
+//
+// A new directory is created on each call to this function, it is recommended to save this result
+// and use it for all tests in a run. For example see BaseSuite.GetRootOutputDir().
+//
+// See runner.GetProfile().GetOutputDir() for the root output directory selection logic.
+//
+// See CreateTestOutputDir and BaseSuite.CreateTestOutputDir for a function that returns a subdirectory for a specific test.
+func CreateRootOutputDir() (string, error) {
+	outputRoot, err := runner.GetProfile().GetOutputDir()
+	if err != nil {
+		return "", err
+	}
+	// Append timestamp to distinguish between multiple runs
+	// Format: YYYY-MM-DD_HH-MM-SS
+	// Use a custom timestamp format because Windows paths can't contain ':' characters
+	// and we don't need the timezone information.
+	timePart := time.Now().Format("2006-01-02_15-04-05")
+	// create root directory
+	err = os.MkdirAll(outputRoot, 0755)
+	if err != nil {
+		return "", err
+	}
+	// Create final output directory
+	// Use MkdirTemp to avoid name collisions between parallel runs
+	outputRoot, err = os.MkdirTemp(outputRoot, fmt.Sprintf("%s_*", timePart))
+	if err != nil {
+		return "", err
+	}
+	if os.Getenv("CI") == "" {
+		// Create a symlink to the latest run for user convenience
+		// TODO: Is there a standard "ci" vs "local" check?
+		//       This code used to be in localProfile.GetOutputDir()
+		latestLink := filepath.Join(filepath.Dir(outputRoot), "latest")
+		// Remove the symlink if it already exists
+		if _, err := os.Lstat(latestLink); err == nil {
+			err = os.Remove(latestLink)
+			if err != nil {
+				return "", err
+			}
+		}
+		err = os.Symlink(outputRoot, latestLink)
+		if err != nil {
+			return "", err
+		}
+	}
+	return outputRoot, nil
+}
+
+// CreateTestOutputDir creates a directory for a specific test that can be used to store output files and artifacts.
+// The test name is used in the directory name, and invalid characters are replaced with underscores.
+//
+// Example:
+//   - test name: TestInstallSuite/TestInstall/install_version=7.50.0
+//   - output directory: <root>/TestInstallSuite/TestInstall/install_version_7_50_0
+func CreateTestOutputDir(root string, t *testing.T) (string, error) {
+	// https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words
+	invalidPathChars := strings.Join([]string{"?", "%", "*", ":", "|", "\"", "<", ">", ".", ",", ";", "="}, "")
+
+	testPart := strings.ReplaceAll(t.Name(), invalidPathChars, "_")
+	path := filepath.Join(root, testPart)
+	err := os.MkdirAll(path, 0755)
+	if err != nil {
+		return "", err
+	}
+	return path, nil
+}

test/new-e2e/pkg/runner/local_profile.go

@@ -10,7 +10,6 @@ import (
 	"os"
 	"os/user"
 	"path"
-	"path/filepath"
 	"strings"
 
 	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner/parameters"
@@ -119,27 +118,3 @@ func (p localProfile) NamePrefix() string {
 func (p localProfile) AllowDevMode() bool {
 	return true
 }
-
-// GetOutputDir extends baseProfile.GetOutputDir to create a symlink to the latest run
-func (p localProfile) GetOutputDir() (string, error) {
-	outDir, err := p.baseProfile.GetOutputDir()
-	if err != nil {
-		return "", err
-	}
-
-	// Create a symlink to the latest run for user convenience
-	latestLink := filepath.Join(filepath.Dir(outDir), "latest")
-	// Remove the symlink if it already exists
-	if _, err := os.Lstat(latestLink); err == nil {
-		err = os.Remove(latestLink)
-		if err != nil {
-			return "", err
-		}
-	}
-	err = os.Symlink(outDir, latestLink)
-	if err != nil {
-		return "", err
-	}
-
-	return outDir, nil
-}

test/new-e2e/pkg/runner/profile.go

@@ -16,11 +16,8 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"time"
 
 	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner/parameters"
-
-	"testing"
 )
 
 // CloudProvider alias to string
@@ -64,9 +61,9 @@ type Profile interface {
 	// AllowDevMode returns if DevMode is allowed
 	AllowDevMode() bool
 	// GetOutputDir returns the root output directory for tests to store output files and artifacts.
-	// e.g. /tmp/e2e-output/2020-01-01_00-00-00_<random>
+	// e.g. /tmp/e2e-output/ or ~/e2e-output/
 	//
-	// See GetTestOutputDir for a function that returns a subdirectory for a specific test.
+	// It is recommended to use GetTestOutputDir to create a subdirectory for a specific test.
 	GetOutputDir() (string, error)
 }
 
@@ -78,7 +75,6 @@ type baseProfile struct {
 	secretStore             parameters.Store
 	workspaceRootFolder     string
 	defaultOutputRootFolder string
-	outputRootFolder        string
 }
 
 func newProfile(projectName string, environments []string, store parameters.Store, secretStore *parameters.Store, defaultOutputRoot string) baseProfile {
@@ -140,55 +136,30 @@ func (p baseProfile) SecretStore() parameters.Store {
 	return p.secretStore
 }
 
-// GetOutputDir returns the root output directory for tests to store output files and artifacts.
-// The directory is created on the first call to this function, normally this will be when a
-// test calls GetTestOutputDir.
+// GetOutputDir returns the root output directory to be used to store output files and artifacts.
+// A path is returned but the directory is not created.
 //
 // The root output directory is chosen in the following order:
 //   - outputDir parameter from the runner configuration, or E2E_OUTPUT_DIR environment variable
-//   - default provided by a parent profile, <defaultOutputRootFolder>/e2e-output, e.g. $CI_PROJECT_DIR/e2e-output
+//   - default provided by profile, <defaultOutputRootFolder>/e2e-output, e.g. $CI_PROJECT_DIR/e2e-output
 //   - os.TempDir()/e2e-output
 //
-// A timestamp is appended to the root output directory to distinguish between multiple runs,
-// and os.MkdirTemp() is used to avoid name collisions between parallel runs.
-//
 // See GetTestOutputDir for a function that returns a subdirectory for a specific test.
 func (p baseProfile) GetOutputDir() (string, error) {
-	if p.outputRootFolder == "" {
-		var outputRoot string
-		configOutputRoot, err := p.store.GetWithDefault(parameters.OutputDir, "")
-		if err != nil {
-			return "", err
-		}
-		if configOutputRoot != "" {
-			// If outputRoot is provided in the config file, use it as the root directory
-			outputRoot = configOutputRoot
-		} else if p.defaultOutputRootFolder != "" {
-			// If a default outputRoot was provided, use it as the root directory
-			outputRoot = filepath.Join(p.defaultOutputRootFolder, "e2e-output")
-		} else if outputRoot == "" {
-			// If outputRoot is not provided, use os.TempDir() as the root directory
-			outputRoot = filepath.Join(os.TempDir(), "e2e-output")
-		}
-		// Append timestamp to distinguish between multiple runs
-		// Format: YYYY-MM-DD_HH-MM-SS
-		// Use a custom timestamp format because Windows paths can't contain ':' characters
-		// and we don't need the timezone information.
-		timePart := time.Now().Format("2006-01-02_15-04-05")
-		// create root directory
-		err = os.MkdirAll(outputRoot, 0755)
-		if err != nil {
-			return "", err
-		}
-		// Create final output directory
-		// Use MkdirTemp to avoid name collisions between parallel runs
-		outputRoot, err = os.MkdirTemp(outputRoot, fmt.Sprintf("%s_*", timePart))
-		if err != nil {
-			return "", err
-		}
-		p.outputRootFolder = outputRoot
+	configOutputRoot, err := p.store.GetWithDefault(parameters.OutputDir, "")
+	if err != nil {
+		return "", err
 	}
-	return p.outputRootFolder, nil
+	if configOutputRoot != "" {
+		// If outputRoot is provided in the config file, use it as the root directory
+		return configOutputRoot, nil
+	}
+	if p.defaultOutputRootFolder != "" {
+		// If a default outputRoot was provided, use it as the root directory
+		return filepath.Join(p.defaultOutputRootFolder, "e2e-output"), nil
+	}
+	// as a final fallback, use os.TempDir() as the root directory
+	return filepath.Join(os.TempDir(), "e2e-output"), nil
 }
 
 // GetWorkspacePath returns the directory for CI Pulumi workspace.
@@ -222,21 +193,3 @@ func GetProfile() Profile {
 
 	return runProfile
 }
-
-// GetTestOutputDir returns the output directory for a specific test.
-// The test name is sanitized to remove invalid characters, and the output directory is created.
-func GetTestOutputDir(p Profile, t *testing.T) (string, error) {
-	// https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words
-	invalidPathChars := strings.Join([]string{"?", "%", "*", ":", "|", "\"", "<", ">", ".", ",", ";", "="}, "")
-	root, err := p.GetOutputDir()
-	if err != nil {
-		return "", err
-	}
-	testPart := strings.ReplaceAll(t.Name(), invalidPathChars, "_")
-	path := filepath.Join(root, testPart)
-	err = os.MkdirAll(path, 0755)
-	if err != nil {
-		return "", err
-	}
-	return path, nil
-}

test/new-e2e/pkg/utils/e2e/client/agent_client.go

@@ -20,7 +20,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e"
-	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner"
 	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/e2e/client/agentclient"
 	"github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/e2e/client/agentclientparams"
 )
@@ -197,13 +196,15 @@ func waitForReadyTimeout(t *testing.T, host *Host, commandRunner *agentCommandRu
 }
 
 func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, host *Host) error {
-	profile := runner.GetProfile()
-	outputDir, err := profile.GetOutputDir()
-	flareFound := false
-
+	root, err := e2e.CreateRootOutputDir()
 	if err != nil {
-		return fmt.Errorf("could not get output directory: %v", err)
+		return fmt.Errorf("could not get root output directory: %w", err)
 	}
+	outputDir, err := e2e.CreateTestOutputDir(root, t)
+	if err != nil {
+		return fmt.Errorf("could not get output directory: %w", err)
+	}
+	flareFound := false
 
 	_, err = commandRunner.FlareWithError(agentclient.WithArgs([]string{"--email", "e2e@test.com", "--send", "--local"}))
 	if err != nil {
@@ -213,17 +214,17 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h
 
 	flareRegex, err := regexp.Compile(`datadog-agent-.*\.zip`)
 	if err != nil {
-		return fmt.Errorf("could not compile regex: %v", err)
+		return fmt.Errorf("could not compile regex: %w", err)
 	}
 
 	tmpFolder, err := host.GetTmpFolder()
 	if err != nil {
-		return fmt.Errorf("could not get tmp folder: %v", err)
+		return fmt.Errorf("could not get tmp folder: %w", err)
 	}
 
 	entries, err := host.ReadDir(tmpFolder)
 	if err != nil {
-		return fmt.Errorf("could not read directory: %v", err)
+		return fmt.Errorf("could not read directory: %w", err)
 	}
 
 	for _, entry := range entries {
@@ -233,15 +234,15 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h
 			if host.osFamily != osComp.WindowsFamily {
 				_, err = host.Execute(fmt.Sprintf("sudo chmod 744 %s/%s", tmpFolder, entry.Name()))
 				if err != nil {
-					return fmt.Errorf("could not update permission of flare file %s/%s : %v", tmpFolder, entry.Name(), err)
+					return fmt.Errorf("could not update permission of flare file %s/%s : %w", tmpFolder, entry.Name(), err)
 				}
 			}
 
 			t.Logf("Downloading flare file in: %s", outputDir)
 			err = host.GetFile(fmt.Sprintf("%s/%s", tmpFolder, entry.Name()), fmt.Sprintf("%s/%s", outputDir, entry.Name()))
 
 			if err != nil {
-				return fmt.Errorf("could not download flare file from %s/%s : %v", tmpFolder, entry.Name(), err)
+				return fmt.Errorf("could not download flare file from %s/%s : %w", tmpFolder, entry.Name(), err)
 			}
 
 			flareFound = true
@@ -253,21 +254,21 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h
 
 		logsFolder, err := host.GetLogsFolder()
 		if err != nil {
-			return fmt.Errorf("could not get logs folder: %v", err)
+			return fmt.Errorf("could not get logs folder: %w", err)
 		}
 
 		entries, err = host.ReadDir(logsFolder)
 
 		if err != nil {
-			return fmt.Errorf("could not read directory: %v", err)
+			return fmt.Errorf("could not read directory: %w", err)
 		}
 
 		for _, entry := range entries {
 			t.Logf("Found log file: %s. Downloading file in: %s", entry.Name(), outputDir)
 
 			err = host.GetFile(fmt.Sprintf("%s/%s", logsFolder, entry.Name()), fmt.Sprintf("%s/%s", outputDir, entry.Name()))
 			if err != nil {
-				return fmt.Errorf("could not download log file from %s/%s : %v", logsFolder, entry.Name(), err)
+				return fmt.Errorf("could not download log file from %s/%s : %w", logsFolder, entry.Name(), err)
 			}
 		}
 	}