From a4d97cc47d1b3dcf95a06736e38ac8f3b98d8247 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Tue, 13 Aug 2019 04:50:20 -0700 Subject: [PATCH] kubectl: eliminate unnecessary blank ending line (#81229) * kubectl: eliminate unnecessary blank ending line * address comments and add more tests --- pkg/kubectl/cmd/get/get.go | 41 +++++++++++++------ pkg/kubectl/cmd/get/get_test.go | 71 +++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/pkg/kubectl/cmd/get/get.go b/pkg/kubectl/cmd/get/get.go index d62605c8128..c4c1355d29d 100644 --- a/pkg/kubectl/cmd/get/get.go +++ b/pkg/kubectl/cmd/get/get.go @@ -521,11 +521,10 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e // track if we write any output trackingWriter := &trackingWriterWrapper{Delegate: o.Out} + // output an empty line separating output + separatorWriter := &separatorWriterWrapper{Delegate: trackingWriter} - // remember how much we've written - written := 0 - - w := utilprinters.GetNewTabWriter(trackingWriter) + w := utilprinters.GetNewTabWriter(separatorWriter) for ix := range objs { var mapping *meta.RESTMapping var info *resource.Info @@ -547,14 +546,13 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e w.Flush() w.SetRememberedWidths(nil) - // add linebreak between resource groups (if there is more than one) - // skip linebreak above first resource group - if lastMapping != nil && !o.NoHeaders { - // If we've written output since the last time we started a new set of headers, write an empty line to separate object types - if written != trackingWriter.Written { - fmt.Fprintln(w) - written = trackingWriter.Written - } + // add linebreaks between resource groups (if there is more than one) + // when it satisfies all following 3 conditions: + // 1) it's not the first resource group + // 2) it has row header + // 3) we've written output since the last time we started a new set of headers + if lastMapping != nil && !o.NoHeaders && trackingWriter.Written > 0 { + separatorWriter.SetReady(true) } printer, err = o.ToPrinter(mapping, nil, printWithNamespace, printWithKind) @@ -600,6 +598,25 @@ func (t *trackingWriterWrapper) Write(p []byte) (n int, err error) { return t.Delegate.Write(p) } +type separatorWriterWrapper struct { + Delegate io.Writer + Ready bool +} + +func (s *separatorWriterWrapper) Write(p []byte) (n int, err error) { + // If we're about to write non-empty bytes and `s` is ready, + // we prepend an empty line to `p` and reset `s.Read`. + if len(p) != 0 && s.Ready { + fmt.Fprintln(s.Delegate) + s.Ready = false + } + return s.Delegate.Write(p) +} + +func (s *separatorWriterWrapper) SetReady(state bool) { + s.Ready = state +} + // watch starts a client-side watch of one or more resources. // TODO: remove the need for arguments here. func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) error { diff --git a/pkg/kubectl/cmd/get/get_test.go b/pkg/kubectl/cmd/get/get_test.go index d02e4d8c7f7..f097ff63164 100644 --- a/pkg/kubectl/cmd/get/get_test.go +++ b/pkg/kubectl/cmd/get/get_test.go @@ -442,6 +442,77 @@ service/baz ClusterIP } } +func TestNoBlankLinesForGetMultipleTableResource(t *testing.T) { + pods, svcs, _ := cmdtesting.TestData() + + tf := cmdtesting.NewTestFactory().WithNamespace("test") + defer tf.Cleanup() + + codec := scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) + tf.UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case p == "/namespaces/test/pods" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: podTableObjBody(codec, pods.Items...)}, nil + case p == "/namespaces/test/replicationcontrollers" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/services" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: serviceTableObjBody(codec, svcs.Items...)}, nil + case p == "/namespaces/test/statefulsets" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/horizontalpodautoscalers" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/jobs" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/cronjobs" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/daemonsets" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/deployments" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + case p == "/namespaces/test/replicasets" && m == "GET": + return &http.Response{StatusCode: 200, Header: cmdtesting.DefaultHeader(), Body: emptyTableObjBody(codec)}, nil + + default: + t.Fatalf("request url: %#v,and request: %#v", req.URL, req) + return nil, nil + } + }), + } + + streams, _, buf, bufErr := genericclioptions.NewTestIOStreams() + cmd := NewCmdGet("kubectl", tf, streams) + cmd.SetOutput(buf) + + expected := `NAME READY STATUS RESTARTS AGE +pod/foo 0/0 0 +pod/bar 0/0 0 + +NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE +service/baz ClusterIP +` + for _, cmdArgs := range [][]string{ + {"pods,services,jobs"}, + {"deployments,pods,statefulsets,services,jobs"}, + {"all"}, + } { + cmd.Run(cmd, cmdArgs) + + if e, a := expected, buf.String(); e != a { + t.Errorf("[kubectl get %v] expected\n%v\ngot\n%v", cmdArgs, e, a) + } + + // The error out should be empty + if e, a := "", bufErr.String(); e != a { + t.Errorf("[kubectl get %v] expected\n%v\ngot\n%v", cmdArgs, e, a) + } + + buf.Reset() + bufErr.Reset() + } +} + func TestNoBlankLinesForGetAll(t *testing.T) { tf := cmdtesting.NewTestFactory().WithNamespace("test") defer tf.Cleanup()