Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: implement diff command #82

Merged
merged 3 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 174 additions & 0 deletions cmd/diff.go
Original file line number Diff line number Diff line change
@@ -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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed with respect to the Python client is the difference section ordering:

$ reana-client diff events.1 test | grep "^==>"
==> Differences in workflow inputs
==> Differences in workflow outputs
==> Differences in workflow version
==> Differences in workflow workspace
==> Differences in workflow specification
==> Differences in workflow workspace

$ $ ./reana-client-go diff events.1 test | grep "==>"
==> Differences in workflow specification
==> Differences in workflow inputs
==> Differences in workflow outputs
==> Differences in workflow version
==> Differences in workflow workspace
==> Differences in workflow workspace

Some observations:

  • It's weird that there are two "workflow workspace" sections, but that's also true for the Python client, and should be fixed there. So nothing to do here.
  • Can we align the output order to respect Python client?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ordering is different because GO's maps are unordered, unlike in Python. It's possible to maintain order by using json.Decoder instead of the json.Unmarshal being used. The code will probably be a little bit more verbose but the change shouldn't be too difficult

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we do it now or after package refactoring?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do it now

Copy link
Member Author

@BrunoRosendo BrunoRosendo Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out I was wrong about using the Decoder, it works nicely for arrays but not so much for objects, since they don't guarantee order by nature. By looking at this discussion I found some hacky workarounds but no nice solution for this, the developers don't seem to support it because one should use an array for order.

An example of a not so nice solution is using the Decoder and manually parsing the JSON, it doesn't look good at all: https://go.dev/play/p/jOXpSDpJz04

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's worth implementing this order (if we want order, shouldn't the server return it as an array?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, fully agreed that it would be very clean to do this on the server-side. But since it would necessitate breaking REST API response changes, it's not something for now.

Hmm, looking at the pasted workaround, it might do actually as a stop-gap solution; however it's also possible that we can live with the Python client <-> Go client output differences for this commend, since it is not probable that some clients has wrapped it into some wider scripted glue usage scenarios... I'm kind of 60:40 split on this.

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
}

utils.PrintColorable(line, out, lineColor)
fmt.Fprintln(out)
}
}
171 changes: 171 additions & 0 deletions cmd/diff_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
})
}
}
1 change: 1 addition & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func NewRootCmd() *cobra.Command {
cmd.AddCommand(newLogsCmd())
cmd.AddCommand(newStatusCmd())
cmd.AddCommand(newLsCmd())
cmd.AddCommand(newDiffCmd())

return cmd
}
Expand Down
8 changes: 1 addition & 7 deletions utils/display_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
13 changes: 13 additions & 0 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading