Skip to content

Commit 11acf57

Browse files
committed
Address copilot review comments
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
1 parent d9311b7 commit 11acf57

2 files changed

Lines changed: 28 additions & 12 deletions

File tree

thirdparty/cmdconfig/commands/cmdcat/cmdcat.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2019-2026 The Kpt Authors.
1+
// Copyright 2019-2026 The kpt Authors.
22
// SPDX-License-Identifier: Apache-2.0
33

44
package cmdcat
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/kptdev/kpt/internal/docs/generated/pkgdocs"
1616
kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1"
17+
"github.com/kptdev/kpt/pkg/printer"
1718
"github.com/kptdev/kpt/thirdparty/cmdconfig/commands/runner"
1819
"github.com/spf13/cobra"
1920
"sigs.k8s.io/kustomize/kyaml/kio"
@@ -24,8 +25,7 @@ import (
2425
// GetCatRunner returns a command CatRunner.
2526
func GetCatRunner(ctx context.Context, _ string) *CatRunner {
2627
r := &CatRunner{
27-
Ctx: ctx,
28-
errWriter: os.Stderr,
28+
Ctx: ctx,
2929
}
3030
c := &cobra.Command{
3131
Use: "cat [FILE | DIR]",
@@ -63,7 +63,6 @@ type CatRunner struct {
6363
Styles []string
6464
StripComments bool
6565
RecurseSubPackages bool
66-
errWriter io.Writer
6766
}
6867

6968
func (r *CatRunner) runE(c *cobra.Command, args []string) error {
@@ -76,6 +75,9 @@ func (r *CatRunner) runE(c *cobra.Command, args []string) error {
7675
switch strings.ToLower(filepath.Ext(args[0])) {
7776
case ".yaml", ".yml", ".json":
7877
default:
78+
if filepath.Base(args[0]) == kptfilev1.KptFileName {
79+
return fmt.Errorf("%q is a %s package metadata file, not a resource file; no resources will be read", args[0], kptfilev1.KptFileName)
80+
}
7981
return fmt.Errorf("%q is not a YAML/JSON file; no resources will be read", args[0])
8082
}
8183
}
@@ -114,7 +116,8 @@ func (r *CatRunner) ExecuteCmd(w io.Writer, pkgPath string) error {
114116
if err != nil {
115117
// Emit a contextual diagnostic on stderr so the user knows which
116118
// package failed; the runner currently aborts on the first error.
117-
fmt.Fprintf(r.errWriter, "kpt pkg cat: %s in package %q\n", err.Error(), pkgPath)
119+
fmt.Fprintf(printer.FromContextOrDie(r.Ctx).ErrStream(),
120+
"kpt pkg cat: %s in package %q\n", err.Error(), pkgPath)
118121
return err
119122
}
120123
fmt.Fprint(w, out.String())

thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ package cmdcat
66
import (
77
"bytes"
88
"context"
9+
"io"
910
"os"
1011
"path/filepath"
1112
"strings"
1213
"testing"
1314

15+
"github.com/kptdev/kpt/pkg/printer"
1416
"github.com/stretchr/testify/assert"
1517
)
1618

@@ -28,7 +30,8 @@ func writeFile(t *testing.T, path, content string) {
2830
func runCat(t *testing.T, args ...string) (string, error) {
2931
t.Helper()
3032
b := &bytes.Buffer{}
31-
r := GetCatRunner(context.Background(), "")
33+
ctx := printer.WithContext(context.Background(), printer.New(nil, io.Discard))
34+
r := GetCatRunner(ctx, "")
3235
r.Command.SetArgs(args)
3336
r.Command.SetOut(b)
3437
r.Command.SetErr(&bytes.Buffer{})
@@ -323,11 +326,18 @@ metadata:
323326
for _, arg := range []string{d, d + "/", "./" + filepath.Base(d)} {
324327
t.Run(arg, func(t *testing.T) {
325328
// Run from d's parent so "./<name>" resolves.
326-
oldWd, _ := os.Getwd()
329+
oldWd, err := os.Getwd()
330+
if err != nil {
331+
t.Fatal(err)
332+
}
327333
if err := os.Chdir(filepath.Dir(d)); err != nil {
328334
t.Fatal(err)
329335
}
330-
defer os.Chdir(oldWd)
336+
defer func() {
337+
if err := os.Chdir(oldWd); err != nil {
338+
t.Fatal(err)
339+
}
340+
}()
331341

332342
got, err := runCat(t, arg)
333343
assert.NoError(t, err)
@@ -389,10 +399,12 @@ data:
389399
assert.Error(t, err)
390400
assert.Contains(t, err.Error(), "broken.yaml")
391401

392-
// Stderr must carry the contextual diagnostic.
402+
// Stderr must carry the contextual diagnostic. Inject a printer via
403+
// context (mirroring how run.go wires the root cobra err stream) and
404+
// assert on its err stream.
393405
var errBuf bytes.Buffer
394-
r := GetCatRunner(context.Background(), "")
395-
r.errWriter = &errBuf
406+
ctx := printer.WithContext(context.Background(), printer.New(nil, &errBuf))
407+
r := GetCatRunner(ctx, "")
396408
r.Command.SetArgs([]string{d})
397409
r.Command.SetOut(&bytes.Buffer{})
398410
r.Command.SetErr(&bytes.Buffer{})
@@ -422,7 +434,8 @@ metadata:
422434
`)
423435
_, err := runCat(t, kpt)
424436
assert.Error(t, err)
425-
assert.Contains(t, err.Error(), "not a YAML/JSON file")
437+
assert.Contains(t, err.Error(), "Kptfile")
438+
assert.Contains(t, err.Error(), "metadata")
426439
}
427440

428441
// TestCmd_NonYAMLFileErrors: a regular non-YAML/JSON/Kptfile file must

0 commit comments

Comments
 (0)