From 1d5d97925d14ba21bf2a9a0b97556e3997b20e83 Mon Sep 17 00:00:00 2001 From: Bruno Rosendo Date: Wed, 17 Aug 2022 10:02:47 +0200 Subject: [PATCH 1/3] cli: implement diff command closes #32 --- cmd/diff.go | 174 ++++++++++++++++++++++++++++++++++++++++++ cmd/root.go | 1 + utils/display_test.go | 8 +- utils/utils.go | 13 ++++ 4 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 cmd/diff.go diff --git a/cmd/diff.go b/cmd/diff.go new file mode 100644 index 0000000..12e1478 --- /dev/null +++ b/cmd/diff.go @@ -0,0 +1,174 @@ +/* +This file is part of REANA. +Copyright (C) 2022 CERN. + +REANA is free software; you can redistribute it and/or modify it +under the terms of the MIT License; see LICENSE file for more details. +*/ + +package cmd + +import ( + "encoding/json" + "fmt" + "io" + "reanahub/reana-client-go/client" + "reanahub/reana-client-go/client/operations" + "reanahub/reana-client-go/utils" + + "github.com/jedib0t/go-pretty/v6/text" + + "github.com/spf13/cobra" +) + +const diffDesc = ` +Show diff between two workflows. + +The ` + "``diff``" + ` command allows to compare two workflows, the workflow_a and +workflow_b, which must be provided as arguments. The output will show the +difference in workflow run parameters, the generated files, the logs, etc. + +Examples: + + $ reana-client diff myanalysis.42 myotheranalysis.43 + + $ reana-client diff myanalysis.42 myotheranalysis.43 --brief +` + +type diffOptions struct { + token string + workflowA string + workflowB string + brief bool + unified int +} + +// newDiffCmd creates a command to show diff between two workflows. +func newDiffCmd() *cobra.Command { + o := &diffOptions{} + + cmd := &cobra.Command{ + Use: "diff", + Short: "Show diff between two workflows.", + Long: diffDesc, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + o.workflowA = args[0] + o.workflowB = args[1] + return o.run(cmd) + }, + } + + f := cmd.Flags() + f.StringVarP(&o.token, "access-token", "t", "", "Access token of the current user.") + f.BoolVarP(&o.brief, "brief", "q", false, `If not set, differences in the contents of the +files in the two workspaces are shown.`) + f.IntVarP( + &o.unified, "unified", "u", 5, "Sets number of context lines for workspace diff output.", + ) + + return cmd +} + +func (o *diffOptions) run(cmd *cobra.Command) error { + diffParams := operations.NewGetWorkflowDiffParams() + diffParams.SetAccessToken(&o.token) + diffParams.SetWorkflowIDOrNamea(o.workflowA) + diffParams.SetWorkflowIDOrNameb(o.workflowB) + diffParams.SetBrief(&o.brief) + contextLines := fmt.Sprintf("%d", o.unified) + diffParams.SetContextLines(&contextLines) + + api, err := client.ApiClient() + if err != nil { + return err + } + diffResp, err := api.Operations.GetWorkflowDiff(diffParams) + if err != nil { + return err + } + + err = displayDiffPayload(cmd, diffResp.Payload) + if err != nil { + return err + } + + return nil +} + +func displayDiffPayload(cmd *cobra.Command, p *operations.GetWorkflowDiffOKBody) error { + leadingMark := "==>" + + if p.ReanaSpecification != "" { + var specificationDiff map[string][]string + err := json.Unmarshal([]byte(p.ReanaSpecification), &specificationDiff) + if err != nil { + return err + } + + // Rename section workflow to specification + val, hasWorkflow := specificationDiff["workflow"] + if hasWorkflow { + specificationDiff["specification"] = val + delete(specificationDiff, "workflow") + } + equalSpecification := true + for section, lines := range specificationDiff { + if len(lines) != 0 { + equalSpecification = false + utils.PrintColorable( + fmt.Sprintf("%s Differences in workflow %s\n", leadingMark, section), + cmd.OutOrStdout(), + text.FgYellow, + text.Bold, + ) + printDiff(lines, cmd.OutOrStdout()) + } + } + if equalSpecification { + utils.PrintColorable( + fmt.Sprintf("%s No differences in REANA specifications.\n", leadingMark), + cmd.OutOrStdout(), + text.FgYellow, + text.Bold, + ) + } + cmd.Println() // Separation line + } + + var workspaceDiffRaw string + err := json.Unmarshal([]byte(p.WorkspaceListing), &workspaceDiffRaw) + if err != nil { + return err + } + if workspaceDiffRaw != "" { + workspaceDiff := utils.SplitLinesNoEmpty(workspaceDiffRaw) + + utils.PrintColorable( + fmt.Sprintf("%s Differences in workflow workspace\n", leadingMark), + cmd.OutOrStdout(), + text.FgYellow, + text.Bold, + ) + printDiff(workspaceDiff, cmd.OutOrStdout()) + } + + return nil +} + +func printDiff(lines []string, out io.Writer) { + for _, line := range lines { + lineColor := text.Reset + switch line[0] { + case '@': + lineColor = text.FgCyan + case '-': + lineColor = text.FgRed + case '+': + lineColor = text.FgGreen + } + + line = fmt.Sprintf("%s\n", line) + utils.PrintColorable(line, out, lineColor) + } +} diff --git a/cmd/root.go b/cmd/root.go index 5f4e874..0bd65a8 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -55,6 +55,7 @@ func NewRootCmd() *cobra.Command { cmd.AddCommand(newLogsCmd()) cmd.AddCommand(newStatusCmd()) cmd.AddCommand(newLsCmd()) + cmd.AddCommand(newDiffCmd()) return cmd } diff --git a/utils/display_test.go b/utils/display_test.go index 968d892..1986e08 100644 --- a/utils/display_test.go +++ b/utils/display_test.go @@ -51,13 +51,7 @@ func TestDisplayTable(t *testing.T) { DisplayTable(test.headers, test.rows, buf) result := buf.String() - splitFn := func(c rune) bool { - return c == '\n' - } // Ignores empty string after \n, unlike strings.Split - lines := strings.FieldsFunc( - result, - splitFn, - ) + lines := SplitLinesNoEmpty(result) if len(lines) != len(test.rows)+1 { t.Fatalf("Expected %d table lines, got %d", len(test.rows)+1, len(lines)) } diff --git a/utils/utils.go b/utils/utils.go index efd24db..b0aa2c6 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -170,3 +170,16 @@ func BindViperToCmdFlag(f *pflag.Flag) error { } return nil } + +// SplitLinesNoEmpty splits a given string into a list where each line is a list item. +// In contrary to strings.Split, SplitLinesNoEmpty ignores empty lines. +func SplitLinesNoEmpty(str string) []string { + splitFn := func(c rune) bool { + return c == '\n' + } // Ignores empty string after \n, unlike strings.Split + lines := strings.FieldsFunc( + str, + splitFn, + ) + return lines +} From cf03ad45ec7ac5f839cc26f40dbeec6691b61ac1 Mon Sep 17 00:00:00 2001 From: Bruno Rosendo Date: Wed, 17 Aug 2022 11:37:52 +0200 Subject: [PATCH 2/3] tests: add tests for utils.SplitLinesNoEmpty --- utils/utils_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/utils/utils_test.go b/utils/utils_test.go index 26a0795..56bd393 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -248,3 +248,23 @@ func TestHandleApiError(t *testing.T) { } } } + +func TestSplitLinesNoEmpty(t *testing.T) { + tests := []struct { + arg string + want []string + }{ + {arg: "", want: []string{}}, + {arg: "a", want: []string{"a"}}, + {arg: "a\nb", want: []string{"a", "b"}}, + {arg: "a\nb\nc", want: []string{"a", "b", "c"}}, + {arg: "a\nb\nc\n", want: []string{"a", "b", "c"}}, + {arg: "a\n\nb\n\nc", want: []string{"a", "b", "c"}}, + } + for _, test := range tests { + got := SplitLinesNoEmpty(test.arg) + if !slices.Equal(got, test.want) { + t.Errorf("Expected %v, got %v", test.want, got) + } + } +} From 3a28223fcb9084bc48166ea3b52ff1d7f469e27b Mon Sep 17 00:00:00 2001 From: Bruno Rosendo Date: Wed, 17 Aug 2022 11:38:18 +0200 Subject: [PATCH 3/3] tests: add unit tests for diff command closes #32 --- cmd/diff.go | 2 +- cmd/diff_test.go | 171 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 cmd/diff_test.go diff --git a/cmd/diff.go b/cmd/diff.go index 12e1478..f616a42 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -168,7 +168,7 @@ func printDiff(lines []string, out io.Writer) { lineColor = text.FgGreen } - line = fmt.Sprintf("%s\n", line) utils.PrintColorable(line, out, lineColor) + fmt.Fprintln(out) } } diff --git a/cmd/diff_test.go b/cmd/diff_test.go new file mode 100644 index 0000000..0a3d3bc --- /dev/null +++ b/cmd/diff_test.go @@ -0,0 +1,171 @@ +/* +This file is part of REANA. +Copyright (C) 2022 CERN. + +REANA is free software; you can redistribute it and/or modify it +under the terms of the MIT License; see LICENSE file for more details. +*/ + +package cmd + +import ( + "bytes" + "fmt" + "net/http" + "reanahub/reana-client-go/utils" + "testing" + + "github.com/jedib0t/go-pretty/v6/text" +) + +var diffPathTemplate = "/api/workflows/%s/diff/%s" + +func TestDiff(t *testing.T) { + workflowA := "my_workflow_a" + workflowB := "my_workflow_b" + diffResponse := `{ + "reana_specification":` + `"{` + + `\"version\": [\"@@ -1 +1 @@\", \"- v0.1\", \"+ v0.2\"],` + + `\"inputs\": [\"@@ -1 +2 @@\", \"- removed input\", \"+ added input\", \"+ more input\"],` + + `\"outputs\": [\"@@ -2 +1 @@\", \"- removed output\", \"- more output\", \"+ added output\"],` + + `\"workflow\": [\"@@ +1 @@\", \"+ added specs\"]` + + `}"` + `, + "workspace_listing": "\"Only in my_workflow_a: test.yaml\"" + }` + sameSpecResponse := `{ + "reana_specification":` + `"{` + + `\"version\": [],\"inputs\": [],\"outputs\": [],\"specification\": []` + + `}"` + `, + "workspace_listing": "\"Only in my_workflow_a: test.yaml\"" + }` + noSpecResponse := `{ + "reana_specification": "", + "workspace_listing": "\"Only in my_workflow_a: test.yaml\"" + }` + noWorkspaceResponse := `{ + "reana_specification":` + `"{` + + `\"version\": [],\"inputs\": [],\"outputs\": [],\"specification\": []` + + `}"` + `, + "workspace_listing": "\"\"" + }` + + tests := map[string]TestCmdParams{ + "all info": { + serverResponse: diffResponse, + statusCode: http.StatusOK, + args: []string{workflowA, workflowB}, + expected: []string{ + "Differences in workflow version", "@@ -1 +1 @@", "- v0.1", "+ v0.2", + "Differences in workflow inputs", "@@ -1 +2 @@", "- removed input", "+ added input", "+ more input", + "Differences in workflow outputs", "@@ -2 +1 @@", "- removed output", "- more output", "+ added output", + "Differences in workflow specification", "@@ +1 @@", "+ added specs", + "Differences in workflow workspace", "Only in my_workflow_a: test.yaml", + }, + }, + "same specification": { + serverResponse: sameSpecResponse, + statusCode: http.StatusOK, + args: []string{workflowA, workflowB}, + expected: []string{ + "No differences in REANA specifications", + "Differences in workflow workspace", "Only in my_workflow_a: test.yaml", + }, + unwanted: []string{ + "Differences in workflow version", "Differences in workflow inputs", + "Differences in workflow specification", "Differences in workflow outputs", + }, + }, + "no specification info": { + serverResponse: noSpecResponse, + statusCode: http.StatusOK, + args: []string{workflowA, workflowB}, + expected: []string{ + "Differences in workflow workspace", "Only in my_workflow_a: test.yaml", + }, + unwanted: []string{ + "No differences in REANA specifications", + "Differences in workflow version", "Differences in workflow inputs", + "Differences in workflow specification", "Differences in workflow outputs", + }, + }, + "no workspace info": { + serverResponse: noWorkspaceResponse, + statusCode: http.StatusOK, + args: []string{workflowA, workflowB}, + expected: []string{ + "No differences in REANA specifications", + }, + unwanted: []string{ + "Differences in workflow workspace", + }, + }, + "unexisting workflow": { + serverResponse: `{"message": "Workflow my_workflow_a does not exist."}`, + statusCode: http.StatusNotFound, + args: []string{workflowA, workflowB}, + expected: []string{"Workflow my_workflow_a does not exist."}, + wantError: true, + }, + "invalid number of arguments": { + args: []string{workflowA}, + expected: []string{"accepts 2 arg(s), received 1"}, + wantError: true, + }, + } + + for name, params := range tests { + t.Run(name, func(t *testing.T) { + params.cmd = "diff" + params.serverPath = fmt.Sprintf(diffPathTemplate, workflowA, workflowB) + testCmdRun(t, params) + }) + } +} + +func TestPrintDiff(t *testing.T) { + tests := map[string]struct { + lines []string + expectedColors []text.Color + }{ + "default text": { + lines: []string{"default text"}, + expectedColors: []text.Color{text.Reset}, + }, + "diff info": { + lines: []string{"@@ -1,14 +1,26 @@"}, + expectedColors: []text.Color{text.FgCyan}, + }, + "removed text": { + lines: []string{"- removed text"}, + expectedColors: []text.Color{text.FgRed}, + }, + "added text": { + lines: []string{"+ added text"}, + expectedColors: []text.Color{text.FgGreen}, + }, + "mixed text": { + lines: []string{"@@ -1 +1 @@", "context", "- removed text", "+ added text"}, + expectedColors: []text.Color{text.FgCyan, text.Reset, text.FgRed, text.FgGreen}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + resBuf := new(bytes.Buffer) + printDiff(test.lines, resBuf) + result := utils.SplitLinesNoEmpty(resBuf.String()) + + if len(result) != len(test.lines) { + t.Fatalf("Expected %d lines, got %d", len(test.lines), len(result)) + } + for i, line := range result { + testBuf := new(bytes.Buffer) + utils.PrintColorable(test.lines[i], testBuf, test.expectedColors[i]) + expected := testBuf.String() + if line != expected { + t.Errorf("Expected %s, got %s", expected, line) + } + } + }) + } +}