fix(e2e/test): stderr stripRE + race-free resolver injection

Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

Author: Jaisheesh-2006

e2e/testdata/fn-render/image-pull-policy-never/.expected/config.yaml

@@ -17,4 +17,4 @@ exitCode: 1
 # The output in stderr is different depending on the runtime.
 # "No such image" is from docker and "image not known" is from podman.
 stdErrRegEx: No such image|image not known
-diffStripRegEx: "\\+\\s+stderr:|docker: Error response from daemon|Run 'docker run|Error:.*image not known|^\\+$"
+diffStripRegEx: "\\+\\s*stderr:|docker: Error response from daemon|Run 'docker run|Error:.*image not known|^\\+$"

e2e/testdata/fn-render/missing-fn-image/.expected/config.yaml

@@ -14,4 +14,4 @@
 
 exitCode: 1
 StdErrRegEx: "(docker: Error response from daemon: Head.*denied|Error.*initializing source docker)"
-diffStripRegEx: "\\+\\s+stderr:|docker: Error response from daemon|Run 'docker run|Error:.*initializing source|^\\+\\s+denied$|^\\+$"
+diffStripRegEx: "\\+\\s*stderr:|docker: Error response from daemon|Run 'docker run|Error:.*initializing source|^\\+\\s*denied$|^\\+$"

pkg/lib/util/parse/parse.go

@@ -35,27 +35,37 @@ type Target struct {
 	Destination string
 }
 
-// defaultBranchResolver resolves the default branch of a remote git repo.
-// Indirected through a package variable so tests can swap in a hermetic
-// stub without making live network calls to github.com (or wherever the
-// repo is hosted).
-var defaultBranchResolver = func(ctx context.Context, repo string) (string, error) {
+// branchResolver looks up the default branch of a remote git repo.
+// Plumbed as a parameter (rather than a package-level global) so tests
+// can supply a hermetic stub without mutating shared state — parallel
+// tests stay race-free.
+type branchResolver func(ctx context.Context, repo string) (string, error)
+
+// defaultBranchResolver is the production branchResolver. It performs a
+// live ls-remote against the repo; unit tests should never use it.
+func defaultBranchResolver(ctx context.Context, repo string) (string, error) {
 	gur, err := gitutil.NewGitUpstreamRepo(ctx, repo)
 	if err != nil {
 		return "", err
 	}
 	return gur.GetDefaultBranch(ctx)
 }
 
+// GitParseArgs is the public entry point; it always uses the live
+// defaultBranchResolver. Tests use gitParseArgs directly with a stub.
 func GitParseArgs(ctx context.Context, args []string, explicitDest bool) (Target, error) {
+	return gitParseArgs(ctx, args, explicitDest, defaultBranchResolver)
+}
+
+func gitParseArgs(ctx context.Context, args []string, explicitDest bool, resolve branchResolver) (Target, error) {
 	g := Target{}
 	if args[0] == "-" {
 		return g, nil
 	}
 
 	// Simple parsing if contains .git{$|/)
 	if HasGitSuffix(args[0]) {
-		return targetFromPkgURL(ctx, args[0], args[1], explicitDest)
+		return targetFromPkgURL(ctx, args[0], args[1], explicitDest, resolve)
 	}
 
 	// GitHub parsing if contains github.com
@@ -64,7 +74,7 @@ func GitParseArgs(ctx context.Context, args []string, explicitDest bool) (Target
 		if err != nil {
 			return g, err
 		}
-		return targetFromPkgURL(ctx, ghPkgURL, args[1], explicitDest)
+		return targetFromPkgURL(ctx, ghPkgURL, args[1], explicitDest, resolve)
 	}
 
 	uri, version, err := getURIAndVersion(args[0])
@@ -76,7 +86,7 @@ func GitParseArgs(ctx context.Context, args []string, explicitDest bool) (Target
 		return g, err
 	}
 	if version == "" {
-		defaultRef, err := defaultBranchResolver(ctx, repo)
+		defaultRef, err := resolve(ctx, repo)
 		if err != nil {
 			return g, err
 		}
@@ -95,7 +105,7 @@ func GitParseArgs(ctx context.Context, args []string, explicitDest bool) (Target
 }
 
 // targetFromPkgURL parses a pkg url and destination into kptfile git info and local destination Target
-func targetFromPkgURL(ctx context.Context, pkgURL string, dest string, explicitDest bool) (Target, error) {
+func targetFromPkgURL(ctx context.Context, pkgURL string, dest string, explicitDest bool, resolve branchResolver) (Target, error) {
 	g := Target{}
 	repo, dir, ref, err := URL(pkgURL)
 	if err != nil {
@@ -105,7 +115,7 @@ func targetFromPkgURL(ctx context.Context, pkgURL string, dest string, explicitD
 		dir = "/"
 	}
 	if ref == "" {
-		defaultRef, err := defaultBranchResolver(ctx, repo)
+		defaultRef, err := resolve(ctx, repo)
 		if err != nil {
 			return g, err
 		}

pkg/lib/util/parse/parse_test.go

@@ -217,16 +217,15 @@ func Test_parseURL(t *testing.T) {
 }
 
 func Test_GitParseArgs(t *testing.T) {
-	// Swap in a hermetic default-branch resolver so the test does not hit
-	// github.com (or any network) to discover HEAD. Without this the test
-	// failed intermittently when the remote returned transient errors
-	// (e.g. HTTP 500 from github.com). The stub mirrors what a real
-	// ls-remote would return for kpt's default branch.
-	origResolver := defaultBranchResolver
-	defaultBranchResolver = func(_ context.Context, _ string) (string, error) {
+	// Use a hermetic stub resolver so the test does not hit github.com
+	// (or any network) to discover HEAD. Without this the test failed
+	// intermittently when the remote returned transient errors (e.g. HTTP
+	// 500 from github.com). The stub is passed via the unexported
+	// gitParseArgs helper — no package-level state is mutated, so tests
+	// remain safe under t.Parallel().
+	stubResolver := func(_ context.Context, _ string) (string, error) {
 		return "main", nil
 	}
-	t.Cleanup(func() { defaultBranchResolver = origResolver })
 
 	tests := map[string]struct {
 		ghURL    string
@@ -269,7 +268,7 @@ func Test_GitParseArgs(t *testing.T) {
 				t.SkipNow()
 			}
 			ctx := printer.WithContext(context.Background(), printer.New(nil, nil))
-			actual, err := GitParseArgs(ctx, []string{test.ghURL, test.expected.Destination}, true)
+			actual, err := gitParseArgs(ctx, []string{test.ghURL, test.expected.Destination}, true, stubResolver)
 			assert.NoError(t, err)
 			assert.Equal(t, test.expected, actual)
 		})

pkg/test/runner/normalize_test.go

@@ -590,3 +590,55 @@ index 1234567..89abcde 100644
 		t.Fatalf("expected configMap lines to stay grouped with set-namespace image line, got:\n%s", got)
 	}
 }
+
+// TestNormalizeDiff_StripREAppliesAfterKptfileIndentStrip pins the contract
+// that per-test diffStripRegEx patterns are evaluated against the
+// post-indent-strip form of Kptfile +/- lines. Test configs (e.g.
+// image-pull-policy-never, missing-fn-image) therefore use `\s*` rather
+// than `\s+` when matching keys like `stderr:` so they survive the
+// Kptfile indent-stripping step. This test locks that behavior in so
+// future normalizer changes can't silently re-break the interaction.
+func TestNormalizeDiff_StripREAppliesAfterKptfileIndentStrip(t *testing.T) {
+	// Raw pre-normalization diff, as produced by git diff against a Kptfile
+	// where the rendered pipeline wrote a multi-line stderr block.
+	input := `diff --git a/Kptfile b/Kptfile
+index 1234567..89abcde 100644
+--- a/Kptfile
++++ b/Kptfile
+@@ -1,4 +1,8 @@
++status:
++  renderStatus:
++    mutationSteps:
++      - image: ghcr.io/kptdev/krm-functions-catalog/dne:latest
++        exitCode: 125
++        stderr: |-
++          Error: ghcr.io/kptdev/krm-functions-catalog/dne:latest: image not known`
+
+	// \s* (not \s+) is the key: after the normalizer strips the leading
+	// whitespace off Kptfile +/- lines, the regex still has to match
+	// `+stderr:` with zero spaces after the `+`.
+	stripRE := `\+\s*stderr:|Error:.*image not known`
+
+	got, err := normalizeDiff(input, stripRE)
+	if err != nil {
+		t.Fatalf("normalizeDiff failed: %v", err)
+	}
+
+	if strings.Contains(got, "stderr:") {
+		t.Fatalf("expected stderr label to be stripped, got:\n%s", got)
+	}
+	if strings.Contains(got, "image not known") {
+		t.Fatalf("expected stderr body (matching Error:.*image not known) to be stripped, got:\n%s", got)
+	}
+
+	// Sanity: the relaxed \s* also matches the pre-indent-strip form, so a
+	// stripRE authored for the raw diff still wins after normalization.
+	stripREStrictPlus := `\+\s+stderr:` // old form with \s+
+	gotStrict, err := normalizeDiff(input, stripREStrictPlus)
+	if err != nil {
+		t.Fatalf("normalizeDiff failed: %v", err)
+	}
+	if !strings.Contains(gotStrict, "stderr:") {
+		t.Fatalf("regression-guard: the tight \\s+ pattern *should* fail to strip the post-indent-strip line (that's the bug we fixed); got:\n%s", gotStrict)
+	}
+}