Split printer tests into table generation then table printing

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{})
+	generator := printers.NewTableGenerator()
-	printer.TableHandler(columns, PrintCustomType)
+	generator.TableHandler(columns, PrintCustomType)
 
 	obj := TestPrintType{"test object"}
-	buffer := &bytes.Buffer{}
+	table, err := generator.GenerateTable(&obj, printers.PrintOptions{})
-	err := printer.PrintObj(&obj, buffer)
 	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{})
+	generator := printers.NewTableGenerator()
-	printer.TableHandler(columns, ErrorPrintHandler)
+	generator.TableHandler(columns, ErrorPrintHandler)
 	obj := TestPrintType{"test object"}
-	buffer := &bytes.Buffer{}
+	_, err := generator.GenerateTable(&obj, printers.PrintOptions{})
-	err := printer.PrintObj(&obj, buffer)
 	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{
+		table, err := generator.GenerateTable(test.obj, options)
-			WithNamespace: true,
+		if err != nil {
-			NoHeaders:     true,
+			t.Fatalf("An error occurred generating table for object: %#v", err)
-		})
+		}
-		AddHandlers(printer)
 		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
+		obj    runtime.Object
-		opts         printers.PrintOptions
+		opts   printers.PrintOptions
-		expect       string
+		expect string
-		ignoreLegacy bool
 	}{
 		{
-			obj: runningPod, opts: printers.PrintOptions{},
+			obj:    runningPod,
+			opts:   printers.PrintOptions{},
 			expect: "NAME    READY   STATUS    RESTARTS   AGE\ntest1   1/2     Running   6          <unknown>\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          <unknown>\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          <unknown>   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          <unknown>   1\ntest2   1/2     Failed    6          <unknown>   \n",
 		},
 		{
-			obj: runningPod, opts: printers.PrintOptions{NoHeaders: true},
+			obj:    runningPod,
+			opts:   printers.PrintOptions{NoHeaders: true},
 			expect: "test1   1/2   Running   6     <unknown>\n",
 		},
 		{
-			obj: failedPod, opts: printers.PrintOptions{},
+			obj:    failedPod,
-			expect:       "NAME    READY   STATUS   RESTARTS   AGE\ntest2   1/2     Failed   6          <unknown>\n",
+			opts:   printers.PrintOptions{},
-			ignoreLegacy: true, // filtering is not done by the printer in the legacy path
+			expect: "NAME    READY   STATUS   RESTARTS   AGE\ntest2   1/2     Failed   6          <unknown>\n",
 		},
 		{
-			obj: failedPod, opts: printers.PrintOptions{},
+			obj:    failedPod,
+			opts:   printers.PrintOptions{},
 			expect: "NAME    READY   STATUS   RESTARTS   AGE\ntest2   1/2     Failed   6          <unknown>\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()))
-		}
 	}
 }
 

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
 	}

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