Graphviz interfaces and unit tests (#106)

* Linter fixes

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Added some coverage

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Added interface for the mehtods used from graphviz

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Fixed the workflow config flag file and bug fixes

Signed-off-by: Prafulla Mahindrakar <[email protected]>
(cherry picked from commit 77be561)

* Added unit tests

Signed-off-by: Prafulla Mahindrakar <[email protected]>

cmd/config/subcommand/workflow/workflow_config.go

@@ -4,7 +4,7 @@ import (
 	"github.com/flyteorg/flytectl/pkg/filters"
 )
 
-//go:generate pflags Config --default-var DefaultConfig
+//go:generate pflags Config --default-var DefaultConfig --bind-default-var
 
 var (
 	DefaultConfig = &Config{
@@ -17,6 +17,4 @@ type Config struct {
 	Version string          `json:"version" pflag:",version of the workflow to be fetched."`
 	Latest  bool            `json:"latest" pflag:", flag to indicate to fetch the latest version, version flag will be ignored in this case"`
 	Filter  filters.Filters `json:"filter" pflag:","`
-	Visualize  string `json:"visualize" pflag:",optional flag to visualize a workflow as one of [png, dot, svg, jpg]"`
-	OutputFile string `json:"output_file" pflag:",path and a filename of where the output image should be dumped. This can only be used in concert with visualize"`
-}
+}

cmd/get/workflow.go

@@ -2,6 +2,7 @@ package get
 
 import (
 	"context"
+
 	workflowconfig "github.com/flyteorg/flytectl/cmd/config/subcommand/workflow"
 	"github.com/flyteorg/flytectl/pkg/ext"
 	"github.com/flyteorg/flytestdlib/logger"

pkg/printer/printer.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"github.com/pkg/browser"
 	"net/url"
 	"os"
 
@@ -17,6 +16,7 @@ import (
 	"github.com/golang/protobuf/proto"
 	"github.com/kataras/tablewriter"
 	"github.com/landoop/tableprinter"
+	"github.com/pkg/browser"
 	"github.com/yalp/jsonpath"
 )
 
@@ -31,7 +31,7 @@ const (
 	OutputFormatDOTURL
 )
 
-const GraphVisualizationServiceUrl = "http://graph.flyte.org/#"
+const GraphVisualizationServiceURL = "http://graph.flyte.org/#"
 
 func OutputFormats() []string {
 	var v []string
@@ -164,8 +164,8 @@ func (p Printer) Print(format OutputFormat, columns []Column, messages ...proto.
 			return errors.Wrapf("VisualizationError", err, "failed to visualize workflow")
 		}
 		if format == OutputFormatDOTURL {
-			urlToOpen := GraphVisualizationServiceUrl + url.PathEscape(graphStr)
-			fmt.Println("Opening the browser at "+ urlToOpen)
+			urlToOpen := GraphVisualizationServiceURL + url.PathEscape(graphStr)
+			fmt.Println("Opening the browser at " + urlToOpen)
 			return browser.OpenURL(urlToOpen)
 		}
 		fmt.Println(graphStr)

pkg/printer/printer_test.go

@@ -2,6 +2,7 @@ package printer
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -27,6 +28,14 @@ func LaunchplanToProtoMessages(l []*admin.LaunchPlan) []proto.Message {
 	return messages
 }
 
+func WorkflowToProtoMessages(l []*admin.Workflow) []proto.Message {
+	messages := make([]proto.Message, 0, len(l))
+	for _, m := range l {
+		messages = append(messages, m)
+	}
+	return messages
+}
+
 // TODO Convert this to a Testable Example. For some reason the comparison fails
 func TestJSONToTable(t *testing.T) {
 	d := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
@@ -57,9 +66,9 @@ func TestJSONToTable(t *testing.T) {
 }
 
 func TestOutputFormats(t *testing.T) {
-	expected := []string{"TABLE", "JSON", "YAML"}
+	expected := []string{"TABLE", "JSON", "YAML", "DOT", "DOTURL"}
 	outputs := OutputFormats()
-	assert.Equal(t, 3, len(outputs))
+	assert.Equal(t, 5, len(outputs))
 	assert.Equal(t, expected, outputs)
 }
 
@@ -77,7 +86,7 @@ func TestOutputFormatStringErr(t *testing.T) {
 }
 
 func TestIsAOutputFormat(t *testing.T) {
-	o := OutputFormat(4)
+	o := OutputFormat(5)
 	check := o.IsAOutputFormat()
 	assert.Equal(t, false, check)
 
@@ -123,4 +132,123 @@ func TestPrint(t *testing.T) {
 	assert.Nil(t, err)
 	err = p.Print(OutputFormat(2), lp, LaunchplanToProtoMessages(launchPlans)...)
 	assert.Nil(t, err)
+	err = p.Print(OutputFormat(3), lp, LaunchplanToProtoMessages(launchPlans)...)
+	assert.NotNil(t, err)
+	err = p.Print(OutputFormat(4), lp, LaunchplanToProtoMessages(launchPlans)...)
+	assert.NotNil(t, err)
+	assert.Equal(t, fmt.Errorf("visualization is only supported on workflows"), err)
+
+	sortedListLiteralType := core.Variable{
+		Type: &core.LiteralType{
+			Type: &core.LiteralType_CollectionType{
+				CollectionType: &core.LiteralType{
+					Type: &core.LiteralType_Simple{
+						Simple: core.SimpleType_INTEGER,
+					},
+				},
+			},
+		},
+	}
+	variableMap := map[string]*core.Variable{
+		"sorted_list1": &sortedListLiteralType,
+		"sorted_list2": &sortedListLiteralType,
+	}
+
+	var compiledTasks []*core.CompiledTask
+	compiledTasks = append(compiledTasks, &core.CompiledTask{
+		Template: &core.TaskTemplate{
+			Id: &core.Identifier{
+				Project: "dummyProject",
+				Domain:  "dummyDomain",
+				Name:    "dummyName",
+				Version: "dummyVersion",
+			},
+			Interface: &core.TypedInterface{
+				Inputs: &core.VariableMap{
+					Variables: variableMap,
+				},
+			},
+		},
+	})
+
+	workflow1 := &admin.Workflow{
+		Id: &core.Identifier{
+			Name:    "task1",
+			Version: "v1",
+		},
+		Closure: &admin.WorkflowClosure{
+			CreatedAt: &timestamppb.Timestamp{Seconds: 1, Nanos: 0},
+			CompiledWorkflow: &core.CompiledWorkflowClosure{
+				Tasks: compiledTasks,
+			},
+		},
+	}
+
+	workflows := []*admin.Workflow{workflow1}
+
+	err = p.Print(OutputFormat(3), lp, WorkflowToProtoMessages(workflows)...)
+	assert.Nil(t, err)
+	workflows = []*admin.Workflow{}
+	err = p.Print(OutputFormat(3), lp, WorkflowToProtoMessages(workflows)...)
+	assert.NotNil(t, err)
+	assert.Equal(t, fmt.Errorf("atleast one workflow required for visualization"), err)
+	var badCompiledTasks []*core.CompiledTask
+	badCompiledTasks = append(badCompiledTasks, &core.CompiledTask{
+		Template: &core.TaskTemplate{},
+	})
+	badWorkflow := &admin.Workflow{
+		Id: &core.Identifier{
+			Name:    "task1",
+			Version: "v1",
+		},
+		Closure: &admin.WorkflowClosure{
+			CreatedAt: &timestamppb.Timestamp{Seconds: 1, Nanos: 0},
+			CompiledWorkflow: &core.CompiledWorkflowClosure{
+				Tasks: badCompiledTasks,
+			},
+		},
+	}
+	workflows = []*admin.Workflow{badWorkflow}
+	err = p.Print(OutputFormat(3), lp, WorkflowToProtoMessages(workflows)...)
+	assert.NotNil(t, err)
+
+	assert.Equal(t, fmt.Errorf("no template found in the workflow task template:<> "), errors.Unwrap(err))
+
+	badWorkflow2 := &admin.Workflow{
+		Id: &core.Identifier{
+			Name:    "task1",
+			Version: "v1",
+		},
+		Closure: &admin.WorkflowClosure{
+			CreatedAt:        &timestamppb.Timestamp{Seconds: 1, Nanos: 0},
+			CompiledWorkflow: nil,
+		},
+	}
+	workflows = []*admin.Workflow{badWorkflow2}
+	err = p.Print(OutputFormat(3), lp, WorkflowToProtoMessages(workflows)...)
+	assert.NotNil(t, err)
+	assert.Equal(t, fmt.Errorf("empty workflow closure"), errors.Unwrap(err))
+
+	var badSubWorkflow []*core.CompiledWorkflow
+	badSubWorkflow = append(badSubWorkflow, &core.CompiledWorkflow{
+		Template: &core.WorkflowTemplate{},
+	})
+
+	badWorkflow3 := &admin.Workflow{
+		Id: &core.Identifier{
+			Name:    "task1",
+			Version: "v1",
+		},
+		Closure: &admin.WorkflowClosure{
+			CreatedAt: &timestamppb.Timestamp{Seconds: 1, Nanos: 0},
+			CompiledWorkflow: &core.CompiledWorkflowClosure{
+				Tasks:        compiledTasks,
+				SubWorkflows: badSubWorkflow,
+			},
+		},
+	}
+	workflows = []*admin.Workflow{badWorkflow3}
+	err = p.Print(OutputFormat(3), lp, WorkflowToProtoMessages(workflows)...)
+	assert.NotNil(t, err)
+	assert.Equal(t, fmt.Errorf("no template found in the sub workflow template:<> "), errors.Unwrap(err))
 }