From 75f70b6a3b2310fe537ed207541450a375d62532 Mon Sep 17 00:00:00 2001 From: "Erik Osterman (CEO @ Cloud Posse)" Date: Tue, 4 Feb 2025 06:15:02 -0600 Subject: [PATCH] Sanitize snapshots (#1002) * sanitize snapshots * [autofix.ci] apply automated fixes * check for empty repo root * normalize slashes * normalize slashes * try to fix windows snapshots with windows paths * handle multiple slashes * handle multiple slashes * changed strategy for removing double slashes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- tests/cli_test.go | 121 +++++++++++++++++- ...mmands_atmos_describe_config.stdout.golden | 10 +- ...tmos_describe_config_-f_yaml.stdout.golden | 10 +- tests/test-cases/demo-stacks.yaml | 12 +- 4 files changed, 125 insertions(+), 28 deletions(-) diff --git a/tests/cli_test.go b/tests/cli_test.go index 79ed0a494..865659339 100644 --- a/tests/cli_test.go +++ b/tests/cli_test.go @@ -16,6 +16,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/creack/pty" + "github.com/go-git/go-git/v5" "github.com/hexops/gotextdiff" "github.com/hexops/gotextdiff/myers" "github.com/hexops/gotextdiff/span" @@ -26,9 +27,11 @@ import ( ) // Command-line flag for regenerating snapshots -var regenerateSnapshots = flag.Bool("regenerate-snapshots", false, "Regenerate all golden snapshots") -var startingDir string -var snapshotBaseDir string +var ( + regenerateSnapshots = flag.Bool("regenerate-snapshots", false, "Regenerate all golden snapshots") + startingDir string + snapshotBaseDir string +) // Define styles using lipgloss var ( @@ -172,7 +175,78 @@ func (pm *PathManager) Apply() error { // Determine if running in a CI environment func isCIEnvironment() bool { - return os.Getenv("CI") != "" + // Check for common CI environment variables + // Note, that the CI variable has many possible truthy values, so we check for any non-empty value that is not "false". + return (os.Getenv("CI") != "" && os.Getenv("CI") != "false") || os.Getenv("GITHUB_ACTIONS") == "true" +} + +// collapseExtraSlashes replaces multiple consecutive slashes with a single slash. +func collapseExtraSlashes(s string) string { + return regexp.MustCompile("/+").ReplaceAllString(s, "/") +} + +// sanitizeOutput replaces occurrences of the repository's absolute path in the output +// with the placeholder "/absolute/path/to/repo". It first normalizes both the repository root +// and the output to use forward slashes, ensuring that the replacement works reliably. +// An error is returned if the repository root cannot be determined. +// Convert something like: +// +// D:\\a\atmos\atmos\examples\demo-stacks\stacks\deploy\**\* +// --> /absolute/path/to/repo/examples/demo-stacks/stacks/deploy/**/* +// /home/runner/work/atmos/atmos/examples/demo-stacks/stacks/deploy/**/* +// --> /absolute/path/to/repo/examples/demo-stacks/stacks/deploy/**/* +func sanitizeOutput(output string) (string, error) { + // 1. Get the repository root. + repoRoot, err := findGitRepoRoot(startingDir) + if err != nil { + return "", err + } + + if repoRoot == "" { + return "", errors.New("failed to determine repository root") + } + + // 2. Normalize the repository root: + // - Clean the path (which may not collapse all extra slashes after the drive letter, etc.) + // - Convert to forward slashes, + // - And explicitly collapse extra slashes. + normalizedRepoRoot := collapseExtraSlashes(filepath.ToSlash(filepath.Clean(repoRoot))) + // Also normalize the output to use forward slashes. + normalizedOutput := filepath.ToSlash(output) + + // 3. Build a regex that matches the repository root even if extra slashes appear. + // First, escape any regex metacharacters in the normalized repository root. + quoted := regexp.QuoteMeta(normalizedRepoRoot) + // Replace each literal "/" with the regex token "/+" so that e.g. "a/b/c" becomes "a/+b/+c". + patternBody := strings.ReplaceAll(quoted, "/", "/+") + // Allow for extra trailing slashes. + pattern := patternBody + "/*" + repoRootRegex, err := regexp.Compile(pattern) + if err != nil { + return "", err + } + + // 4. Replace any occurrence of the repository root (with extra slashes) with a fixed placeholder. + // The placeholder will end with exactly one slash. + placeholder := "/absolute/path/to/repo/" + replaced := repoRootRegex.ReplaceAllString(normalizedOutput, placeholder) + + // 5. Now collapse extra slashes in the remainder of file paths that start with the placeholder. + // We use a regex to find segments that start with the placeholder followed by some path characters. + // (We assume that file paths appear in quotes or other delimited contexts, and that URLs won't match.) + fixRegex := regexp.MustCompile(`(/absolute/path/to/repo)([^",]+)`) + result := fixRegex.ReplaceAllStringFunc(replaced, func(match string) string { + // The regex has two groups: group 1 is the placeholder, group 2 is the remainder. + groups := fixRegex.FindStringSubmatch(match) + if len(groups) < 3 { + return match + } + // Collapse extra slashes in the remainder. + fixedRemainder := collapseExtraSlashes(groups[2]) + return groups[1] + fixedRemainder + }) + + return result, nil } // sanitizeTestName converts t.Name() into a valid filename. @@ -561,11 +635,11 @@ func verifyFileContains(t *testing.T, filePatterns map[string][]MatchPattern) bo } func updateSnapshot(fullPath, output string) { - err := os.MkdirAll(filepath.Dir(fullPath), 0755) // Ensure parent directories exist + err := os.MkdirAll(filepath.Dir(fullPath), 0o755) // Ensure parent directories exist if err != nil { panic(fmt.Sprintf("Failed to create snapshot directory: %v", err)) } - err = os.WriteFile(fullPath, []byte(output), 0644) // Write snapshot + err = os.WriteFile(fullPath, []byte(output), 0o644) // Write snapshot if err != nil { panic(fmt.Sprintf("Failed to write snapshot file: %v", err)) } @@ -645,6 +719,17 @@ func verifySnapshot(t *testing.T, tc TestCase, stdoutOutput, stderrOutput string return true } + // Sanitize outputs and fail the test if sanitization fails. + var err error + stdoutOutput, err = sanitizeOutput(stdoutOutput) + if err != nil { + t.Fatalf("failed to sanitize stdout output: %v", err) + } + stderrOutput, err = sanitizeOutput(stderrOutput) + if err != nil { + t.Fatalf("failed to sanitize stderr output: %v", err) + } + testName := sanitizeTestName(t.Name()) stdoutFileName := fmt.Sprintf("%s.stdout.golden", testName) stderrFileName := fmt.Sprintf("%s.stderr.golden", testName) @@ -675,7 +760,6 @@ $ go test -run=%q -regenerate-snapshots`, stdoutPath, t.Name()) if isCIEnvironment() || !term.IsTerminal(int(os.Stdout.Fd())) { // Generate a colorized diff for better readability diff = generateUnifiedDiff(filteredStdoutActual, filteredStdoutExpected) - } else { diff = colorizeDiffWithThreshold(filteredStdoutActual, filteredStdoutExpected, 10) } @@ -706,6 +790,29 @@ $ go test -run=%q -regenerate-snapshots`, stderrPath, t.Name()) return true } +// findGitRepo finds the Git repository root +func findGitRepoRoot(path string) (string, error) { + // Open the Git repository starting from the given path + repo, err := git.PlainOpenWithOptions(path, &git.PlainOpenOptions{DetectDotGit: true}) + if err != nil { + return "", fmt.Errorf("failed to find git repository: %w", err) + } + + // Get the repository's working tree + worktree, err := repo.Worktree() + if err != nil { + return "", fmt.Errorf("failed to get worktree: %w", err) + } + + // Return the absolute path to the root of the working tree + root, err := filepath.Abs(worktree.Filesystem.Root()) + if err != nil { + return "", fmt.Errorf("failed to get absolute path of repository root: %w", err) + } + + return root, nil +} + func TestUnmarshalMatchPattern(t *testing.T) { yamlData := ` expect: diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden index 3c9b7660e..40e44840c 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden @@ -123,15 +123,15 @@ "base_path": "" }, "initialized": true, - "stacksBaseAbsolutePath": "/Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/stacks", + "stacksBaseAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks/stacks", "includeStackAbsolutePaths": [ - "/Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/stacks/deploy/**/*" + "/absolute/path/to/repo/examples/demo-stacks/stacks/deploy/**/*" ], "excludeStackAbsolutePaths": [ - "/Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/stacks/**/_defaults.yaml" + "/absolute/path/to/repo/examples/demo-stacks/stacks/**/_defaults.yaml" ], - "terraformDirAbsolutePath": "/Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/components/terraform", - "helmfileDirAbsolutePath": "/Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks", + "terraformDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks/components/terraform", + "helmfileDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks", "default": false, "version": { "Check": { diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden index de0d09a67..19b2c2f36 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden @@ -32,13 +32,13 @@ settings: list_merge_strategy: "" inject_github_token: true initialized: true -stacksBaseAbsolutePath: /Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/stacks +stacksBaseAbsolutePath: /absolute/path/to/repo/examples/demo-stacks/stacks includeStackAbsolutePaths: - - /Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/stacks/deploy/**/* + - /absolute/path/to/repo/examples/demo-stacks/stacks/deploy/**/* excludeStackAbsolutePaths: - - /Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/stacks/**/_defaults.yaml -terraformDirAbsolutePath: /Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks/components/terraform -helmfileDirAbsolutePath: /Users/matt/src/github.com/cloudposse/atmos/examples/demo-stacks + - /absolute/path/to/repo/examples/demo-stacks/stacks/**/_defaults.yaml +terraformDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks/components/terraform +helmfileDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks default: false validate: editorconfig: diff --git a/tests/test-cases/demo-stacks.yaml b/tests/test-cases/demo-stacks.yaml index 80923b7be..3fc5c07a3 100644 --- a/tests/test-cases/demo-stacks.yaml +++ b/tests/test-cases/demo-stacks.yaml @@ -96,12 +96,7 @@ tests: - "-f" - "yaml" expect: - diff: - - "stacksBaseAbsolutePath" - - "terraformDirAbsolutePath" - - "helmfileDirAbsolutePath" - - 'examples[/\\]+demo-stacks[/\\]+stacks[/\\]+\*\*[/\\]+_defaults.yaml' - - 'examples[/\\]+demo-stacks[/\\]+stacks[/\\]deploy[/\\]+\*\*[/\\]+\*' + diff: [] stdout: - 'append_user_agent: Atmos/(\d+\.\d+\.\d+|test) \(Cloud Posse; \+https:\/\/atmos\.tools\)' stderr: @@ -120,11 +115,6 @@ tests: expect: diff: - '"append_user_agent": "Atmos/(\d+\.\d+\.\d+|test) \(Cloud Posse; \+https:\/\/atmos\.tools\)"' - - "stacksBaseAbsolutePath" - - "terraformDirAbsolutePath" - - "helmfileDirAbsolutePath" - - 'examples[/\\]+demo-stacks[/\\]+stacks[/\\]+\*\*[/\\]+_defaults.yaml' - - 'examples[/\\]+demo-stacks[/\\]+stacks[/\\]+deploy[/\\]+\*\*[/\\]+\*' stdout: - '"append_user_agent": "Atmos/(\d+\.\d+\.\d+|test) \(Cloud Posse; \+https:\/\/atmos\.tools\)"' stderr: