From 6ed077f3a862a2c91325040f4a08eee878308046 Mon Sep 17 00:00:00 2001 From: Yolean k8s-qa Date: Tue, 28 Apr 2026 08:59:20 +0000 Subject: [PATCH] feat(yconverge): CWD-relative paths in dependency / target progress lines The four progress headers reported the CUE-module-root-relative form, which surprised when the user's CWD is upstream of the module root. Example before this change: $ cd y-cluster $ y-cluster yconverge -k ../ystack/yconverge/itest/example-replace-dependent yconverge dependency yconverge/itest/example-replace ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot `cd` to that path from the user's shell -- it's relative to a module root the user didn't name. After: $ cd y-cluster $ y-cluster yconverge -k ../ystack/yconverge/itest/example-replace-dependent yconverge dependency ../ystack/yconverge/itest/example-replace (same shape as the -k arg; `cd` works) Implementation: a small userPath() helper that wraps filepath.Rel against os.Getwd(). Falls back to the absolute path on lookup failure (rare; cross-drive paths on Windows). The diagnostic zap.Debug log lines keep RelPath(cueRoot, step) -- structured fields meant for log aggregation are more useful in module-relative form, which doesn't depend on cwd-at-log-time. The CUE-module-root projection there carries significance (matches CUE's import-path semantics) and stays. Test coverage: - pkg/yconverge/userpath_test.go: relative-to-cwd common case and the traverse-up `..` case. - e2e/converge_progress_test.go: assertions switched from full string match to substring match on the user-meaningful suffix (/base, /dependent), because tests run from the e2e/ dir and fixtures live in t.TempDir() so the path traverses up through ../../tmp/. Co-Authored-By: Claude Opus 4.7 (1M context) --- e2e/converge_progress_test.go | 23 ++++++++++++------- pkg/yconverge/userpath_test.go | 40 ++++++++++++++++++++++++++++++++++ pkg/yconverge/yconverge.go | 33 +++++++++++++++++++++++----- 3 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 pkg/yconverge/userpath_test.go diff --git a/e2e/converge_progress_test.go b/e2e/converge_progress_test.go index 074dc4d..cb03d3c 100644 --- a/e2e/converge_progress_test.go +++ b/e2e/converge_progress_test.go @@ -95,21 +95,28 @@ step: verify.#Step & { checks: [{ } got := string(out) - wants := []string{ - "yconverge dependency base", - "yconverge converge-mode=replace", - "yconverge target dependent", + // userPath in pkg/yconverge prints CWD-relative paths so the + // shown form matches what -k accepts. Tests run with CWD set + // to the e2e/ package dir, so the resolved path traverses + // up to /tmp//... -- we substring-match the segment + // the user would care about ("/base" / "/dependent") rather + // than pinning the long ../../../tmp/.../ prefix. + wantSubs := []string{ + "yconverge dependency", + "/base\nyconverge converge-mode=replace\n", + "yconverge target", + "/dependent\n", "yconverge check 1/1 exec", } - for _, w := range wants { + for _, w := range wantSubs { if !strings.Contains(got, w) { - t.Errorf("missing progress line %q\nfull output:\n%s", w, got) + t.Errorf("missing progress substring %q\nfull output:\n%s", w, got) } } // Order matters: dependency before target, target before check. - depIdx := strings.Index(got, "yconverge dependency base") - tgtIdx := strings.Index(got, "yconverge target dependent") + depIdx := strings.Index(got, "yconverge dependency") + tgtIdx := strings.Index(got, "yconverge target") chkIdx := strings.Index(got, "yconverge check 1/1 exec") if !(depIdx < tgtIdx && tgtIdx < chkIdx) { t.Errorf("progress lines out of order: dep=%d target=%d check=%d\nfull output:\n%s", diff --git a/pkg/yconverge/userpath_test.go b/pkg/yconverge/userpath_test.go new file mode 100644 index 0000000..a54e3ab --- /dev/null +++ b/pkg/yconverge/userpath_test.go @@ -0,0 +1,40 @@ +package yconverge + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestUserPath_RelativeToCWD: a path under cwd renders as a +// short relative string -- the shape `-k ` accepts and +// the user can `cd` to. +func TestUserPath_RelativeToCWD(t *testing.T) { + tmp := t.TempDir() + t.Chdir(tmp) + got := userPath(filepath.Join(tmp, "base")) + if got != "base" { + t.Fatalf("got %q, want %q", got, "base") + } +} + +// TestUserPath_TraversesUp: a path outside cwd produces the +// `../...` form. Long but still actionable in a shell. +func TestUserPath_TraversesUp(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "a/b"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "x"), 0o755); err != nil { + t.Fatal(err) + } + t.Chdir(filepath.Join(root, "a/b")) + got := userPath(filepath.Join(root, "x")) + if !strings.HasPrefix(got, "..") { + t.Fatalf("got %q, expected `..`-prefixed path", got) + } + if !strings.HasSuffix(got, "/x") { + t.Fatalf("got %q, expected trailing /x", got) + } +} diff --git a/pkg/yconverge/yconverge.go b/pkg/yconverge/yconverge.go index 24dd511..2ce84b5 100644 --- a/pkg/yconverge/yconverge.go +++ b/pkg/yconverge/yconverge.go @@ -38,6 +38,30 @@ func (o Options) progressOut() io.Writer { return os.Stdout } +// userPath turns an absolute filesystem path into something +// readable to the user: a CWD-relative form that matches the +// shape of the -k argument they typed. Tab-completion and a +// follow-up `cd ` Just Work. +// +// Falls back to the absolute path when filepath.Rel can't +// compute one (rare; happens across drive letters on Windows +// or when the cwd is otherwise unrelated to the target). +// +// The diagnostic zap log lines keep RelPath(cueRoot, step) -- +// structured fields meant for log aggregation are more useful +// in module-relative form, which doesn't depend on cwd-at-log-time. +func userPath(absPath string) string { + cwd, err := os.Getwd() + if err != nil { + return absPath + } + rel, err := filepath.Rel(cwd, absPath) + if err != nil { + return absPath + } + return rel +} + // Result holds the outcome of a yconverge run. type Result struct { // Steps lists the directories that were converged, in order. @@ -111,9 +135,8 @@ func Run(ctx context.Context, opts Options, logger *zap.Logger) (*Result, error) zap.Int("steps", len(steps)), ) for _, step := range steps[:len(steps)-1] { - rel := RelPath(cueRoot, step) - logger.Debug("converge dependency", zap.String("dir", rel)) - fmt.Fprintf(opts.progressOut(), "yconverge dependency %s\n", rel) + logger.Debug("converge dependency", zap.String("dir", RelPath(cueRoot, step))) + fmt.Fprintf(opts.progressOut(), "yconverge dependency %s\n", userPath(step)) depOpts := Options{ Context: opts.Context, KustomizeDir: step, @@ -127,7 +150,7 @@ func Run(ctx context.Context, opts Options, logger *zap.Logger) (*Result, error) Stdout: opts.Stdout, } if _, err := convergeSingle(ctx, depOpts, logger); err != nil { - return nil, fmt.Errorf("dependency %s: %w", rel, err) + return nil, fmt.Errorf("dependency %s: %w", userPath(step), err) } } } @@ -137,7 +160,7 @@ func Run(ctx context.Context, opts Options, logger *zap.Logger) (*Result, error) // header for what the user explicitly passed via -k. logger.Debug("converge target", zap.String("dir", RelPath(cueRoot, absDir))) if hasDeps { - fmt.Fprintf(opts.progressOut(), "yconverge target %s\n", RelPath(cueRoot, absDir)) + fmt.Fprintf(opts.progressOut(), "yconverge target %s\n", userPath(absDir)) } if _, err := convergeSingle(ctx, opts, logger); err != nil { return nil, err