From 374eff070c77105032bc813e41b89ca61148add0 Mon Sep 17 00:00:00 2001 From: Sean Sullivan Date: Tue, 13 Aug 2019 21:37:43 -0700 Subject: [PATCH] Split printer tests into table generation then table printing --- pkg/printers/internalversion/printers_test.go | 266 +++++++++--------- pkg/printers/tablegenerator.go | 2 + pkg/printers/tableprinter.go | 5 - 3 files changed, 135 insertions(+), 138 deletions(-) diff --git a/pkg/printers/internalversion/printers_test.go b/pkg/printers/internalversion/printers_test.go index 04ad3833609..b18e30a7410 100644 --- a/pkg/printers/internalversion/printers_test.go +++ b/pkg/printers/internalversion/printers_test.go @@ -301,15 +301,22 @@ func ErrorPrintHandler(obj *TestPrintType, options printers.PrintOptions) ([]met func TestCustomTypePrinting(t *testing.T) { columns := []metav1beta1.TableColumnDefinition{{Name: "Data"}} - printer := printers.NewTablePrinter(printers.PrintOptions{}) - printer.TableHandler(columns, PrintCustomType) + generator := printers.NewTableGenerator() + generator.TableHandler(columns, PrintCustomType) obj := TestPrintType{"test object"} - buffer := &bytes.Buffer{} - err := printer.PrintObj(&obj, buffer) + table, err := generator.GenerateTable(&obj, printers.PrintOptions{}) if err != nil { - t.Fatalf("An error occurred printing the custom type: %#v", err) + t.Fatalf("An error occurred generating the table for custom type: %#v", err) } + + printer := printers.NewTablePrinter(printers.PrintOptions{}) + buffer := &bytes.Buffer{} + err = printer.PrintObj(table, buffer) + if err != nil { + t.Fatalf("An error occurred printing the Table: %#v", err) + } + expectedOutput := "DATA\ntest object\n" if buffer.String() != expectedOutput { t.Errorf("The data was not printed as expected. Expected:\n%s\nGot:\n%s", expectedOutput, buffer.String()) @@ -318,11 +325,10 @@ func TestCustomTypePrinting(t *testing.T) { func TestPrintHandlerError(t *testing.T) { columns := []metav1beta1.TableColumnDefinition{{Name: "Data"}} - printer := printers.NewTablePrinter(printers.PrintOptions{}) - printer.TableHandler(columns, ErrorPrintHandler) + generator := printers.NewTableGenerator() + generator.TableHandler(columns, ErrorPrintHandler) obj := TestPrintType{"test object"} - buffer := &bytes.Buffer{} - err := printer.PrintObj(&obj, buffer) + _, err := generator.GenerateTable(&obj, printers.PrintOptions{}) if err == nil || err.Error() != "ErrorPrintHandler error" { t.Errorf("Did not get the expected error: %#v", err) } @@ -587,34 +593,9 @@ func TestPrinters(t *testing.T) { } } } - - // a humanreadable printer deals with internal-versioned objects - humanReadablePrinter := map[string]printers.ResourcePrinter{ - "humanReadable": printers.NewTablePrinter(printers.PrintOptions{ - NoHeaders: true, - }), - "humanReadableHeaders": printers.NewTablePrinter(printers.PrintOptions{}), - } - AddHandlers((humanReadablePrinter["humanReadable"]).(*printers.HumanReadablePrinter)) - AddHandlers((humanReadablePrinter["humanReadableHeaders"]).(*printers.HumanReadablePrinter)) - for pName, p := range humanReadablePrinter { - for oName, obj := range objects { - b := &bytes.Buffer{} - if err := p.PrintObj(obj, b); err != nil { - if set, found := expectedErrors[pName]; found && set.Has(oName) { - // expected error - continue - } - t.Errorf("printer '%v', object '%v'; error: '%v'", pName, oName, err) - } - } - } } func TestPrintEventsResultSorted(t *testing.T) { - // Arrange - printer := printers.NewTablePrinter(printers.PrintOptions{}) - AddHandlers(printer) obj := api.EventList{ Items: []api.Event{ @@ -644,22 +625,26 @@ func TestPrintEventsResultSorted(t *testing.T) { }, }, } - buffer := &bytes.Buffer{} // Act - err := printer.PrintObj(&obj, buffer) + table, err := printers.NewTableGenerator().With(AddHandlers).GenerateTable(&obj, printers.PrintOptions{}) + if err != nil { + t.Fatalf("An error occurred generating the Table: %#v", err) + } + printer := printers.NewTablePrinter(printers.PrintOptions{}) + buffer := &bytes.Buffer{} + err = printer.PrintObj(table, buffer) // Assert if err != nil { - t.Fatalf("An error occurred printing the EventList: %#v", err) + t.Fatalf("An error occurred printing the Table: %#v", err) } out := buffer.String() VerifyDatesInOrder(out, "\n" /* rowDelimiter */, " " /* columnDelimiter */, t) } func TestPrintNodeStatus(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{}) - AddHandlers(printer) + table := []struct { node api.Node status string @@ -735,9 +720,15 @@ func TestPrintNodeStatus(t *testing.T) { }, } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(printers.PrintOptions{}) for _, test := range table { + table, err := generator.GenerateTable(&test.node, printers.PrintOptions{}) + if err != nil { + t.Fatalf("An error occurred printing Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -748,8 +739,7 @@ func TestPrintNodeStatus(t *testing.T) { } func TestPrintNodeRole(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{}) - AddHandlers(printer) + table := []struct { node api.Node expected string @@ -780,9 +770,15 @@ func TestPrintNodeRole(t *testing.T) { }, } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(printers.PrintOptions{}) for _, test := range table { + table, err := generator.GenerateTable(&test.node, printers.PrintOptions{}) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -793,11 +789,6 @@ func TestPrintNodeRole(t *testing.T) { } func TestPrintNodeOSImage(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{ - ColumnLabels: []string{}, - Wide: true, - }) - AddHandlers(printer) table := []struct { node api.Node @@ -825,9 +816,18 @@ func TestPrintNodeOSImage(t *testing.T) { }, } + options := printers.PrintOptions{ + Wide: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for _, test := range table { + table, err := generator.GenerateTable(&test.node, options) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -838,11 +838,6 @@ func TestPrintNodeOSImage(t *testing.T) { } func TestPrintNodeKernelVersion(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{ - ColumnLabels: []string{}, - Wide: true, - }) - AddHandlers(printer) table := []struct { node api.Node @@ -870,9 +865,18 @@ func TestPrintNodeKernelVersion(t *testing.T) { }, } + options := printers.PrintOptions{ + Wide: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for _, test := range table { + table, err := generator.GenerateTable(&test.node, options) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -883,11 +887,6 @@ func TestPrintNodeKernelVersion(t *testing.T) { } func TestPrintNodeContainerRuntimeVersion(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{ - ColumnLabels: []string{}, - Wide: true, - }) - AddHandlers(printer) table := []struct { node api.Node @@ -915,9 +914,18 @@ func TestPrintNodeContainerRuntimeVersion(t *testing.T) { }, } + options := printers.PrintOptions{ + Wide: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for _, test := range table { + table, err := generator.GenerateTable(&test.node, options) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -928,10 +936,7 @@ func TestPrintNodeContainerRuntimeVersion(t *testing.T) { } func TestPrintNodeName(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{ - Wide: true, - }) - AddHandlers(printer) + table := []struct { node api.Node Name string @@ -952,9 +957,18 @@ func TestPrintNodeName(t *testing.T) { }, } + options := printers.PrintOptions{ + Wide: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for _, test := range table { + table, err := generator.GenerateTable(&test.node, options) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -965,10 +979,7 @@ func TestPrintNodeName(t *testing.T) { } func TestPrintNodeExternalIP(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{ - Wide: true, - }) - AddHandlers(printer) + table := []struct { node api.Node externalIP string @@ -1000,9 +1011,18 @@ func TestPrintNodeExternalIP(t *testing.T) { }, } + options := printers.PrintOptions{ + Wide: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for _, test := range table { + table, err := generator.GenerateTable(&test.node, options) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -1013,10 +1033,7 @@ func TestPrintNodeExternalIP(t *testing.T) { } func TestPrintNodeInternalIP(t *testing.T) { - printer := printers.NewTablePrinter(printers.PrintOptions{ - Wide: true, - }) - AddHandlers(printer) + table := []struct { node api.Node internalIP string @@ -1048,9 +1065,18 @@ func TestPrintNodeInternalIP(t *testing.T) { }, } + options := printers.PrintOptions{ + Wide: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for _, test := range table { + table, err := generator.GenerateTable(&test.node, options) + if err != nil { + t.Fatalf("An error occurred generating table for Node: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(&test.node, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing Node: %#v", err) } @@ -1399,12 +1425,6 @@ func TestPrintHumanReadableWithNamespace(t *testing.T) { }, isNamespaced: true, }, - { - obj: &api.LimitRange{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespaceName}, - }, - isNamespaced: true, - }, { obj: &api.ResourceQuota{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespaceName}, @@ -1419,36 +1439,22 @@ func TestPrintHumanReadableWithNamespace(t *testing.T) { }, isNamespaced: false, }, - { - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "kind": "Foo", - "apiVersion": "example.com/v1", - "metadata": map[string]interface{}{"name": "test", "namespace": namespaceName}, - }, - }, - isNamespaced: true, - }, - { - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "kind": "Foo", - "apiVersion": "example.com/v1", - "metadata": map[string]interface{}{"name": "test"}, - }, - }, - isNamespaced: false, - }, } + //*******// + options := printers.PrintOptions{ + WithNamespace: true, + NoHeaders: true, + } + generator := printers.NewTableGenerator().With(AddHandlers) + printer := printers.NewTablePrinter(options) for i, test := range table { - printer := printers.NewTablePrinter(printers.PrintOptions{ - WithNamespace: true, - NoHeaders: true, - }) - AddHandlers(printer) + table, err := generator.GenerateTable(test.obj, options) + if err != nil { + t.Fatalf("An error occurred generating table for object: %#v", err) + } buffer := &bytes.Buffer{} - err := printer.PrintObj(test.obj, buffer) + err = printer.PrintObj(table, buffer) if err != nil { t.Fatalf("An error occurred printing object: %#v", err) } @@ -1488,38 +1494,43 @@ func TestPrintPodTable(t *testing.T) { }, } tests := []struct { - obj runtime.Object - opts printers.PrintOptions - expect string - ignoreLegacy bool + obj runtime.Object + opts printers.PrintOptions + expect string }{ { - obj: runningPod, opts: printers.PrintOptions{}, + obj: runningPod, + opts: printers.PrintOptions{}, expect: "NAME READY STATUS RESTARTS AGE\ntest1 1/2 Running 6 \n", }, { - obj: runningPod, opts: printers.PrintOptions{WithKind: true, Kind: schema.GroupKind{Kind: "Pod"}}, + obj: runningPod, + opts: printers.PrintOptions{WithKind: true, Kind: schema.GroupKind{Kind: "Pod"}}, expect: "NAME READY STATUS RESTARTS AGE\npod/test1 1/2 Running 6 \n", }, { - obj: runningPod, opts: printers.PrintOptions{ShowLabels: true}, + obj: runningPod, + opts: printers.PrintOptions{ShowLabels: true}, expect: "NAME READY STATUS RESTARTS AGE LABELS\ntest1 1/2 Running 6 a=1,b=2\n", }, { - obj: &api.PodList{Items: []api.Pod{*runningPod, *failedPod}}, opts: printers.PrintOptions{ColumnLabels: []string{"a"}}, + obj: &api.PodList{Items: []api.Pod{*runningPod, *failedPod}}, + opts: printers.PrintOptions{ColumnLabels: []string{"a"}}, expect: "NAME READY STATUS RESTARTS AGE A\ntest1 1/2 Running 6 1\ntest2 1/2 Failed 6 \n", }, { - obj: runningPod, opts: printers.PrintOptions{NoHeaders: true}, + obj: runningPod, + opts: printers.PrintOptions{NoHeaders: true}, expect: "test1 1/2 Running 6 \n", }, { - obj: failedPod, opts: printers.PrintOptions{}, - expect: "NAME READY STATUS RESTARTS AGE\ntest2 1/2 Failed 6 \n", - ignoreLegacy: true, // filtering is not done by the printer in the legacy path + obj: failedPod, + opts: printers.PrintOptions{}, + expect: "NAME READY STATUS RESTARTS AGE\ntest2 1/2 Failed 6 \n", }, { - obj: failedPod, opts: printers.PrintOptions{}, + obj: failedPod, + opts: printers.PrintOptions{}, expect: "NAME READY STATUS RESTARTS AGE\ntest2 1/2 Failed 6 \n", }, } @@ -1531,24 +1542,13 @@ func TestPrintPodTable(t *testing.T) { } verifyTable(t, table) buf := &bytes.Buffer{} - p := printers.NewTablePrinter(test.opts).With(AddHandlers) + p := printers.NewTablePrinter(test.opts) if err := p.PrintObj(table, buf); err != nil { t.Fatal(err) } if test.expect != buf.String() { t.Errorf("%d mismatch:\n%s\n%s", i, strconv.Quote(test.expect), strconv.Quote(buf.String())) } - if test.ignoreLegacy { - continue - } - - buf.Reset() - if err := p.PrintObj(test.obj, buf); err != nil { - t.Fatal(err) - } - if test.expect != buf.String() { - t.Errorf("%d legacy mismatch:\n%s\n%s", i, strconv.Quote(test.expect), strconv.Quote(buf.String())) - } } } diff --git a/pkg/printers/tablegenerator.go b/pkg/printers/tablegenerator.go index 7b39f183ab7..ecfe7574d9e 100644 --- a/pkg/printers/tablegenerator.go +++ b/pkg/printers/tablegenerator.go @@ -115,6 +115,8 @@ func (h *HumanReadableGenerator) GenerateTable(obj runtime.Object, options Print table.SelfLink = m.GetSelfLink() } } + // TODO(seans3): Remove the following decorateTable call. This should only be + // called in the table printer. if err := decorateTable(table, options); err != nil { return nil, err } diff --git a/pkg/printers/tableprinter.go b/pkg/printers/tableprinter.go index 9aba99246eb..883772ed7e3 100644 --- a/pkg/printers/tableprinter.go +++ b/pkg/printers/tableprinter.go @@ -128,8 +128,6 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er h.printedHeaders = true } - // TODO(seans3): Remove the following decorateTable call. Table modification - // (and creation) should only happen in table generation (tablegenerator.go). if err := decorateTable(table, localOptions); err != nil { return err } @@ -286,9 +284,6 @@ func addColumns(pos columnAddPosition, table *metav1beta1.Table, columns []metav return nil } -// TODO(seans3): This method modifies the table, to it should only happen -// during table generation (tablegenerator.go), and not during table printing. -// // decorateTable takes a table and attempts to add label columns and the // namespace column. It will fill empty columns with nil (if the object // does not expose metadata). It returns an error if the table cannot