From 4e93514cf325c90126d042f179dae7b729127891 Mon Sep 17 00:00:00 2001 From: Girish Ramnani Date: Thu, 10 Oct 2019 02:53:03 +0530 Subject: [PATCH 1/3] show indented json for machine readable output --- pkg/machineoutput/types.go | 9 +++++++-- pkg/odo/cli/application/describe.go | 4 ++-- pkg/odo/cli/application/list.go | 6 +++--- pkg/odo/cli/component/describe.go | 4 ++-- pkg/odo/cli/component/list.go | 6 +++--- pkg/odo/cli/project/list.go | 4 ++-- pkg/odo/cli/storage/create.go | 4 ++-- pkg/odo/cli/storage/list.go | 4 ++-- pkg/odo/cli/url/list.go | 4 ++-- pkg/util/util.go | 4 ++-- 10 files changed, 27 insertions(+), 22 deletions(-) diff --git a/pkg/machineoutput/types.go b/pkg/machineoutput/types.go index 7afb1a6f56b..cb288505c04 100644 --- a/pkg/machineoutput/types.go +++ b/pkg/machineoutput/types.go @@ -32,7 +32,7 @@ type GenericSuccess struct { // OutputSuccess outputs a "successful" machine-readable output format in json func OutputSuccess(machineOutput interface{}) { - printableOutput, err := json.Marshal(machineOutput) + printableOutput, err := MarshalJSONIndented(machineOutput) // If we error out... there's no way to output it (since we disable logging when using -o json) if err != nil { @@ -44,7 +44,7 @@ func OutputSuccess(machineOutput interface{}) { // OutputError outputs a "successful" machine-readable output format in json func OutputError(machineOutput interface{}) { - printableOutput, err := json.Marshal(machineOutput) + printableOutput, err := MarshalJSONIndented(machineOutput) // If we error out... there's no way to output it (since we disable logging when using -o json) if err != nil { @@ -53,3 +53,8 @@ func OutputError(machineOutput interface{}) { fmt.Fprintf(log.GetStderr(), "%s\n", string(printableOutput)) } } + +// MarshalJSONIndented returns indented json representation of obj +func MarshalJSONIndented(obj interface{}) ([]byte, error) { + return json.MarshalIndent(obj, "", " ") +} diff --git a/pkg/odo/cli/application/describe.go b/pkg/odo/cli/application/describe.go index 09c28feef59..10764528a1b 100644 --- a/pkg/odo/cli/application/describe.go +++ b/pkg/odo/cli/application/describe.go @@ -1,12 +1,12 @@ package application import ( - "encoding/json" "fmt" "github.com/openshift/odo/pkg/application" "github.com/openshift/odo/pkg/component" "github.com/openshift/odo/pkg/log" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/cli/project" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util" @@ -65,7 +65,7 @@ func (o *DescribeOptions) Validate() (err error) { func (o *DescribeOptions) Run() (err error) { if log.IsJSON() { appDef := application.GetMachineReadableFormat(o.Client, o.appName, o.Project) - out, err := json.Marshal(appDef) + out, err := machineoutput.MarshalJSONIndented(appDef) if err != nil { return err } diff --git a/pkg/odo/cli/application/list.go b/pkg/odo/cli/application/list.go index 71640101efe..3d17efae372 100644 --- a/pkg/odo/cli/application/list.go +++ b/pkg/odo/cli/application/list.go @@ -1,13 +1,13 @@ package application import ( - "encoding/json" "fmt" "os" "text/tabwriter" "github.com/openshift/odo/pkg/application" "github.com/openshift/odo/pkg/log" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/cli/project" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util" @@ -68,7 +68,7 @@ func (o *ListOptions) Run() (err error) { } appListDef := application.GetMachineReadableFormatForList(appList) - out, err := json.Marshal(appListDef) + out, err := machineoutput.MarshalJSONIndented(appListDef) if err != nil { return err } @@ -91,7 +91,7 @@ func (o *ListOptions) Run() (err error) { } } else { if log.IsJSON() { - out, err := json.Marshal(application.GetMachineReadableFormatForList([]application.App{})) + out, err := machineoutput.MarshalJSONIndented(application.GetMachineReadableFormatForList([]application.App{})) if err != nil { return err } diff --git a/pkg/odo/cli/component/describe.go b/pkg/odo/cli/component/describe.go index 67428bb7f61..f3fbc825c7d 100644 --- a/pkg/odo/cli/component/describe.go +++ b/pkg/odo/cli/component/describe.go @@ -1,11 +1,11 @@ package component import ( - "encoding/json" "fmt" "github.com/openshift/odo/pkg/config" "github.com/openshift/odo/pkg/log" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/component" @@ -70,7 +70,7 @@ func (do *DescribeOptions) Run() (err error) { } if log.IsJSON() { componentDesc.Spec.Ports = do.localConfigInfo.GetPorts() - out, err := json.Marshal(componentDesc) + out, err := machineoutput.MarshalJSONIndented(componentDesc) if err != nil { return err } diff --git a/pkg/odo/cli/component/list.go b/pkg/odo/cli/component/list.go index 94c39113965..0eca442f359 100644 --- a/pkg/odo/cli/component/list.go +++ b/pkg/odo/cli/component/list.go @@ -1,7 +1,6 @@ package component import ( - "encoding/json" "fmt" "os" "path/filepath" @@ -9,6 +8,7 @@ import ( "github.com/golang/glog" "github.com/openshift/odo/pkg/application" + "github.com/openshift/odo/pkg/machineoutput" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -67,7 +67,7 @@ func (lo *ListOptions) Run() (err error) { return err } if log.IsJSON() { - out, err := json.Marshal(components) + out, err := machineoutput.MarshalJSONIndented(components) if err != nil { return err } @@ -123,7 +123,7 @@ func (lo *ListOptions) Run() (err error) { if log.IsJSON() { - out, err := json.Marshal(components) + out, err := machineoutput.MarshalJSONIndented(components) if err != nil { return err } diff --git a/pkg/odo/cli/project/list.go b/pkg/odo/cli/project/list.go index a163edbcb4c..5fc9e851b21 100644 --- a/pkg/odo/cli/project/list.go +++ b/pkg/odo/cli/project/list.go @@ -1,12 +1,12 @@ package project import ( - "encoding/json" "fmt" "os" "text/tabwriter" "github.com/openshift/odo/pkg/log" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/project" "github.com/spf13/cobra" @@ -53,7 +53,7 @@ func (plo *ProjectListOptions) Run() (err error) { return err } if log.IsJSON() { - out, err := json.Marshal(projects) + out, err := machineoutput.MarshalJSONIndented(projects) if err != nil { return err } diff --git a/pkg/odo/cli/storage/create.go b/pkg/odo/cli/storage/create.go index ab987c87883..fb4dc17c891 100644 --- a/pkg/odo/cli/storage/create.go +++ b/pkg/odo/cli/storage/create.go @@ -1,11 +1,11 @@ package storage import ( - "encoding/json" "fmt" "github.com/openshift/odo/pkg/config" "github.com/openshift/odo/pkg/log" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util/completion" "github.com/openshift/odo/pkg/storage" @@ -70,7 +70,7 @@ func (o *StorageCreateOptions) Run() (err error) { storageResultMachineReadable := storage.GetMachineReadableFormat(storageResult.Name, storageResult.Size, storageResult.Path) if log.IsJSON() { - out, err := json.Marshal(storageResultMachineReadable) + out, err := machineoutput.MarshalJSONIndented(storageResultMachineReadable) if err != nil { return err } diff --git a/pkg/odo/cli/storage/list.go b/pkg/odo/cli/storage/list.go index f6314efd21f..e1a163853c6 100644 --- a/pkg/odo/cli/storage/list.go +++ b/pkg/odo/cli/storage/list.go @@ -1,11 +1,11 @@ package storage import ( - "encoding/json" "fmt" "os" "github.com/openshift/odo/pkg/config" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/util/completion" "github.com/openshift/odo/pkg/storage" @@ -70,7 +70,7 @@ func (o *StorageListOptions) Run() (err error) { storageListResultMachineReadable := storage.GetMachineReadableFormatForList(storageListMachineReadable) if log.IsJSON() { - out, err := json.Marshal(storageListResultMachineReadable) + out, err := machineoutput.MarshalJSONIndented(storageListResultMachineReadable) if err != nil { return err } diff --git a/pkg/odo/cli/url/list.go b/pkg/odo/cli/url/list.go index cfb867a221b..8068cb2211b 100644 --- a/pkg/odo/cli/url/list.go +++ b/pkg/odo/cli/url/list.go @@ -1,13 +1,13 @@ package url import ( - "encoding/json" "fmt" "os" "text/tabwriter" "github.com/openshift/odo/pkg/config" "github.com/openshift/odo/pkg/log" + "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util" "github.com/openshift/odo/pkg/odo/util/completion" @@ -63,7 +63,7 @@ func (o *URLListOptions) Run() (err error) { } if log.IsJSON() { - out, err := json.Marshal(urls) + out, err := machineoutput.MarshalJSONIndented(urls) if err != nil { return err } diff --git a/pkg/util/util.go b/pkg/util/util.go index 67781d59793..109cdcab524 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -2,7 +2,6 @@ package util import ( "bufio" - "encoding/json" "fmt" "io" "math/rand" @@ -21,6 +20,7 @@ import ( "github.com/gobwas/glob" "github.com/golang/glog" + "github.com/openshift/odo/pkg/machineoutput" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -590,7 +590,7 @@ func MachineOutput(outputFlag string, resource interface{}) (string, error) { switch outputFlag { case "json": // If `-o json` is provided - out, err = json.Marshal(resource) + out, err = machineoutput.MarshalJSONIndented(resource) } return string(out), err From ffb78207d769f421d41654e4a734450a481e3cef Mon Sep 17 00:00:00 2001 From: Girish Ramnani Date: Thu, 10 Oct 2019 15:26:45 +0530 Subject: [PATCH 2/3] resolved failing tests --- tests/helper/helper_generic.go | 16 ++++++++++++++++ tests/integration/component.go | 13 +++++++++---- tests/integration/project/cmd_project_test.go | 6 ++++-- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/helper/helper_generic.go b/tests/helper/helper_generic.go index 2841978aa03..9f1a6813195 100644 --- a/tests/helper/helper_generic.go +++ b/tests/helper/helper_generic.go @@ -1,6 +1,7 @@ package helper import ( + "encoding/json" "fmt" "math/rand" "strings" @@ -63,3 +64,18 @@ func DontMatchAllInOutput(output string, tonotmatch []string) { Expect(output).ToNot(ContainSubstring(i)) } } + +// Unindented returns the unindented version of the jsonStr passed to it +func Unindented(jsonStr string) (string, error) { + var tmpMap map[string]interface{} + err := json.Unmarshal([]byte(jsonStr), &tmpMap) + if err != nil { + return "", err + } + + obj, err := json.Marshal(tmpMap) + if err != nil { + return "", err + } + return string(obj), err +} diff --git a/tests/integration/component.go b/tests/integration/component.go index 72cf935e0c8..d0e0ce439fc 100644 --- a/tests/integration/component.go +++ b/tests/integration/component.go @@ -108,7 +108,8 @@ func componentTests(args ...string) { contextPath = strings.TrimSpace(context) } desired := fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Not Pushed"}}`, project, contextPath) - actual := helper.CmdShouldPass("odo", append(args, "list", "-o", "json", "--path", filepath.Dir(context))...) + actual, err := helper.Unindented(helper.CmdShouldPass("odo", append(args, "list", "-o", "json", "--path", filepath.Dir(context))...)) + Expect(err).Should(BeNil()) // since the tests are run parallel, there might be many odo component directories in the root folder // so we only check for the presence of the current one Expect(actual).Should(ContainSubstring(desired)) @@ -136,12 +137,16 @@ func componentTests(args ...string) { contextPath2 = strings.TrimSpace(context2) } - actual := helper.CmdShouldPass("odo", append(args, "list", "-o", "json", "--path", filepath.Dir(context))...) + actual, err := helper.Unindented(helper.CmdShouldPass("odo", append(args, "list", "-o", "json", "--path", filepath.Dir(context))...)) + Expect(err).Should(BeNil()) helper.Chdir(context) helper.DeleteDir(context2) helper.DeleteProject(project2) - Expect(actual).Should(ContainSubstring(fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project, contextPath))) - Expect(actual).Should(ContainSubstring(fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"python","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"python","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project2, contextPath2))) + expected := fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project, contextPath) + Expect(actual).Should(ContainSubstring(expected)) + expected = fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"python","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"python","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project2, contextPath2) + Expect(actual).Should(ContainSubstring(expected)) + }) It("should create the component from the branch ref when provided", func() { diff --git a/tests/integration/project/cmd_project_test.go b/tests/integration/project/cmd_project_test.go index 2ba49f5b630..45183ca1338 100644 --- a/tests/integration/project/cmd_project_test.go +++ b/tests/integration/project/cmd_project_test.go @@ -44,8 +44,10 @@ var _ = Describe("odo project command tests", func() { Expect(listOutput).To(ContainSubstring(project)) // project deletion doesn't happen immediately, so we test subset of the string - listOutputJson := helper.CmdShouldPass("odo", "project", "list", "-o", "json") - Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}`)) + listOutputJSON, err := helper.Unindented(helper.CmdShouldPass("odo", "project", "list", "-o", "json")) + Expect(err).Should(BeNil()) + expected := `{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}` + Expect(listOutputJSON).To(ContainSubstring(expected)) }) }) }) From b6804aaee63b32af2e35f7a666fd61ddbae94d53 Mon Sep 17 00:00:00 2001 From: Girish Ramnani Date: Fri, 11 Oct 2019 13:58:57 +0530 Subject: [PATCH 3/3] resolve failing tests #2 --- tests/integration/component.go | 13 ++++++++++--- tests/integration/project/cmd_project_test.go | 5 +++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/integration/component.go b/tests/integration/component.go index d0e0ce439fc..d31645a0743 100644 --- a/tests/integration/component.go +++ b/tests/integration/component.go @@ -107,7 +107,10 @@ func componentTests(args ...string) { } else { contextPath = strings.TrimSpace(context) } - desired := fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Not Pushed"}}`, project, contextPath) + // this orders the json + desired, err := helper.Unindented(fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Not Pushed"}}`, project, contextPath)) + Expect(err).Should(BeNil()) + actual, err := helper.Unindented(helper.CmdShouldPass("odo", append(args, "list", "-o", "json", "--path", filepath.Dir(context))...)) Expect(err).Should(BeNil()) // since the tests are run parallel, there might be many odo component directories in the root folder @@ -142,9 +145,13 @@ func componentTests(args ...string) { helper.Chdir(context) helper.DeleteDir(context2) helper.DeleteProject(project2) - expected := fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project, contextPath) + // this orders the json + expected, err := helper.Unindented(fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"nodejs","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"nodejs","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project, contextPath)) + Expect(err).Should(BeNil()) Expect(actual).Should(ContainSubstring(expected)) - expected = fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"python","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"python","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project2, contextPath2) + // this orders the json + expected, err = helper.Unindented(fmt.Sprintf(`{"kind":"Component","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"python","namespace":"%s","creationTimestamp":null},"spec":{"app":"app","type":"python","source":"./","ports":["8080/TCP"]},"status":{"context":"%s","state":"Pushed"}}`, project2, contextPath2)) + Expect(err).Should(BeNil()) Expect(actual).Should(ContainSubstring(expected)) }) diff --git a/tests/integration/project/cmd_project_test.go b/tests/integration/project/cmd_project_test.go index 45183ca1338..17dde235418 100644 --- a/tests/integration/project/cmd_project_test.go +++ b/tests/integration/project/cmd_project_test.go @@ -39,14 +39,15 @@ var _ = Describe("odo project command tests", func() { }) Context("when running project command app parameter in directory that doesn't contain .odo config directory", func() { - It("should successfully execute list along with machine readable output", func() { + FIt("should successfully execute list along with machine readable output", func() { listOutput := helper.CmdShouldPass("odo", "project", "list") Expect(listOutput).To(ContainSubstring(project)) // project deletion doesn't happen immediately, so we test subset of the string listOutputJSON, err := helper.Unindented(helper.CmdShouldPass("odo", "project", "list", "-o", "json")) Expect(err).Should(BeNil()) - expected := `{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}` + expected, err := helper.Unindented(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}`) + Expect(err).Should(BeNil()) Expect(listOutputJSON).To(ContainSubstring(expected)) }) })