Skip to content

Commit 9f83904

Browse files
authored
Replace multi line formatting to single line formatting for kpt logs (#326)
* Replace MultiLineFormatter with a SingleLineFormatter - Remove \n and \t from render errors - Use single line for function results * Fix upgrade cli unit test * Add unit tests to cover new code * fix duplicated literal and add unit tests for kpt errors.go * Update TestSingleLineFormatter unit test in runner_test.go
1 parent 9fee28c commit 9f83904

17 files changed

Lines changed: 369 additions & 136 deletions

File tree

internal/kpt/errors/errors.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2021 The kpt and Nephio Authors
1+
// Copyright 2021,2025 The kpt and Nephio Authors
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -93,7 +93,7 @@ func (e *Error) Error() string {
9393
var wrappedErr *Error
9494
if As(e.Err, &wrappedErr) {
9595
if !wrappedErr.Zero() {
96-
pad(b, ":\n\t")
96+
pad(b, ": ")
9797
b.WriteString(wrappedErr.Error())
9898
}
9999
} else {

internal/kpt/errors/errors_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2025 The kpt and Nephio Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package errors
16+
17+
import (
18+
"errors"
19+
"strings"
20+
"testing"
21+
22+
"github.com/nephio-project/porch/internal/kpt/types"
23+
)
24+
25+
func TestErrorFormatting(t *testing.T) {
26+
baseErr := errors.New("base error")
27+
28+
e := &Error{
29+
Op: "pkg.get",
30+
Path: types.UniquePath("/workspace/my-pkg"),
31+
Fn: "my-fn",
32+
Repo: "github.com/example/repo",
33+
Class: InvalidParam,
34+
Err: baseErr,
35+
}
36+
37+
got := e.Error()
38+
wantSubstrings := []string{
39+
"pkg.get",
40+
"pkg /workspace/my-pkg",
41+
"fn my-fn",
42+
"repo github.com/example/repo",
43+
"invalid parameter value",
44+
"base error",
45+
}
46+
47+
for _, substr := range wantSubstrings {
48+
if !strings.Contains(got, substr) {
49+
t.Errorf("Expected error string to contain %q, got: %q", substr, got)
50+
}
51+
}
52+
}

internal/kpt/fnruntime/fnerrors.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ type ExecError struct {
4545
func (fe *ExecError) String() string {
4646
var b strings.Builder
4747

48-
errLines := &MultiLineFormatter{
49-
Title: "Stderr",
50-
Lines: strings.Split(fe.Stderr, "\n"),
51-
UseQuote: true,
52-
TruncateOutput: fe.TruncateOutput,
48+
errLine := &SingleLineFormatter{
49+
Title: "[Stderr]",
50+
Lines: strings.Split(fe.Stderr, "\n"),
51+
UseQuote: false,
52+
Separator: ", ",
5353
}
54-
b.WriteString(errLines.String())
54+
b.WriteString(errLine.String())
5555
b.WriteString(fmt.Sprintf(" Exit Code: %d\n", fe.ExitCode))
5656
return b.String()
5757
}

internal/kpt/fnruntime/runner.go

Lines changed: 25 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -449,10 +449,11 @@ func printFnResult(ctx context.Context, fnResult *fnresult.Result, opt *printer.
449449
for _, item := range fnResult.Results {
450450
lines = append(lines, item.String())
451451
}
452-
ri := &MultiLineFormatter{
453-
Title: "Results",
454-
Lines: lines,
455-
TruncateOutput: printer.TruncateOutput,
452+
ri := &SingleLineFormatter{
453+
Title: "[Results]",
454+
Lines: lines,
455+
UseQuote: false,
456+
Separator: ", ",
456457
}
457458
pr.OptPrintf(opt, "%s", ri.String())
458459
}
@@ -470,13 +471,13 @@ func printFnExecErr(ctx context.Context, fnErr *ExecError) {
470471
func printFnStderr(ctx context.Context, stdErr string) {
471472
pr := printer.FromContextOrDie(ctx)
472473
if len(stdErr) > 0 {
473-
errLines := &MultiLineFormatter{
474-
Title: "Stderr",
475-
Lines: strings.Split(stdErr, "\n"),
476-
UseQuote: true,
477-
TruncateOutput: printer.TruncateOutput,
474+
errLine := &SingleLineFormatter{
475+
Title: "Stderr",
476+
Lines: strings.Split(stdErr, "\n"),
477+
UseQuote: false,
478+
Separator: ", ",
478479
}
479-
pr.Printf("%s", errLines.String())
480+
pr.Printf("%s", errLine.String())
480481
}
481482
}
482483

@@ -514,63 +515,27 @@ func enforcePathInvariants(nodes []*yaml.RNode) error {
514515
return nil
515516
}
516517

517-
// MultiLineFormatter knows how to format multiple lines in pretty format
518-
// that can be displayed to an end user.
519-
type MultiLineFormatter struct {
520-
// Title under which lines need to be printed
521-
Title string
522-
523-
// Lines to be printed on the CLI.
524-
Lines []string
525-
526-
// TruncateOuput determines if output needs to be truncated or not.
527-
TruncateOutput bool
528-
529-
// MaxLines to be printed if truncation is enabled.
530-
MaxLines int
531-
532-
// UseQuote determines if line needs to be quoted or not
533-
UseQuote bool
518+
type SingleLineFormatter struct {
519+
Title string // Label for the output
520+
Lines []string // Lines to be joined
521+
UseQuote bool // Whether to quote each line
522+
Separator string // Separator between lines (e.g., comma, space)
534523
}
535524

536-
// String returns multiline string.
537-
func (ri *MultiLineFormatter) String() string {
538-
if ri.MaxLines == 0 {
539-
ri.MaxLines = FnExecErrorTruncateLines
540-
}
525+
func (sf *SingleLineFormatter) String() string {
541526
strInterpolator := "%s"
542-
if ri.UseQuote {
527+
if sf.UseQuote {
543528
strInterpolator = "%q"
544529
}
545530

546-
var b strings.Builder
547-
548-
b.WriteString(fmt.Sprintf(" %s:\n", ri.Title))
549-
lineIndent := strings.Repeat(" ", FnExecErrorIndentation+2)
550-
if !ri.TruncateOutput {
551-
// stderr string should have indentations
552-
for _, s := range ri.Lines {
553-
// suppress newlines to avoid poor formatting
554-
s = strings.ReplaceAll(s, "\n", " ")
555-
b.WriteString(fmt.Sprintf(lineIndent+strInterpolator+"\n", s))
556-
}
557-
return b.String()
558-
}
559-
printedLines := 0
560-
for i, s := range ri.Lines {
561-
if i >= ri.MaxLines {
562-
break
563-
}
564-
// suppress newlines to avoid poor formatting
565-
s = strings.ReplaceAll(s, "\n", " ")
566-
b.WriteString(fmt.Sprintf(lineIndent+strInterpolator+"\n", s))
567-
printedLines++
531+
var formattedLines []string
532+
for _, line := range sf.Lines {
533+
line = strings.ReplaceAll(line, "\n", " ")
534+
line = strings.TrimSpace(line)
535+
formattedLines = append(formattedLines, fmt.Sprintf(strInterpolator, line))
568536
}
569-
truncatedLines := len(ri.Lines) - printedLines
570-
if truncatedLines > 0 {
571-
b.WriteString(fmt.Sprintf(lineIndent+"...(%d line(s) truncated, use '--truncate-output=false' to disable)\n", truncatedLines))
572-
}
573-
return b.String()
537+
538+
return fmt.Sprintf("%s: %s", sf.Title, strings.Join(formattedLines, sf.Separator))
574539
}
575540

576541
func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath types.UniquePath) (*yaml.RNode, error) {

internal/kpt/fnruntime/runner_test.go

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -98,58 +98,55 @@ data: {foo: bar}
9898
})
9999
}
100100
}
101-
102-
func TestMultilineFormatter(t *testing.T) {
101+
func TestSingleLineFormatter(t *testing.T) {
103102
type testcase struct {
104-
ml *MultiLineFormatter
103+
sf *SingleLineFormatter
105104
expected string
106105
}
107106

108107
testcases := map[string]testcase{
109-
"multiline should format lines and truncate": {
110-
ml: &MultiLineFormatter{
111-
Title: "Results",
112-
Lines: []string{
113-
"line-1",
114-
"line-2",
115-
"line-3",
116-
"line-4",
117-
"line-5",
118-
},
119-
MaxLines: 3,
120-
TruncateOutput: true,
108+
"single line without quotes and comma separator": {
109+
sf: &SingleLineFormatter{
110+
Title: "Summary",
111+
Lines: []string{"line1", "line2", "line3"},
112+
UseQuote: false,
113+
Separator: ", ",
121114
},
122-
expected: ` Results:
123-
line-1
124-
line-2
125-
line-3
126-
...(2 line(s) truncated, use '--truncate-output=false' to disable)
127-
`,
115+
expected: `Summary: line1, line2, line3`,
128116
},
129-
"multiline should format without truncate": {
130-
ml: &MultiLineFormatter{
131-
Title: "Results",
132-
Lines: []string{
133-
"line-1",
134-
"line-2",
135-
"line-3",
136-
"line-4",
137-
"line-5",
138-
},
117+
"single line with quotes and space separator": {
118+
sf: &SingleLineFormatter{
119+
Title: "Summary",
120+
Lines: []string{"line1", "line2", "line3"},
121+
UseQuote: true,
122+
Separator: " ",
139123
},
140-
expected: ` Results:
141-
line-1
142-
line-2
143-
line-3
144-
line-4
145-
line-5
146-
`,
124+
expected: `Summary: "line1" "line2" "line3"`,
125+
},
126+
"single line with newline suppression": {
127+
sf: &SingleLineFormatter{
128+
Title: "Summary",
129+
Lines: []string{"line1\n", "line2\nextra", "line3"},
130+
UseQuote: false,
131+
Separator: ", ",
132+
},
133+
expected: `Summary: line1, line2 extra, line3`,
134+
},
135+
"empty lines": {
136+
sf: &SingleLineFormatter{
137+
Title: "Empty",
138+
Lines: []string{},
139+
UseQuote: false,
140+
Separator: ", ",
141+
},
142+
expected: `Empty: `,
147143
},
148144
}
145+
149146
for name, c := range testcases {
150147
c := c
151148
t.Run(name, func(t *testing.T) {
152-
assert.Equal(t, c.expected, c.ml.String())
149+
assert.Equal(t, c.expected, c.sf.String())
153150
})
154151
}
155152
}

pkg/cli/commands/rpkg/push/command.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,11 @@ func (r *runner) printFnResult(fnResult *porchapi.Result, opt *printer.Options)
181181
for _, item := range fnResult.Results {
182182
lines = append(lines, str(item))
183183
}
184-
ri := &fnruntime.MultiLineFormatter{
185-
Title: "Results",
186-
Lines: lines,
187-
TruncateOutput: printer.TruncateOutput,
184+
ri := &fnruntime.SingleLineFormatter{
185+
Title: "[Results]",
186+
Lines: lines,
187+
UseQuote: false,
188+
Separator: ", ",
188189
}
189190
r.printer.OptPrintf(opt, "%s", ri.String())
190191
}

0 commit comments

Comments
 (0)