From 4fed5a836563a9cdf3316de889e6eabd74be1239 Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Thu, 10 Sep 2020 11:17:36 +0530 Subject: [PATCH 1/2] return error instead of nil Signed-off-by: Gaurav Gahlot --- db/template.go | 4 +--- grpc-server/workflow.go | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/db/template.go b/db/template.go index 0f765558d..9e2f07acf 100644 --- a/db/template.go +++ b/db/template.go @@ -56,13 +56,11 @@ func (d TinkDB) GetTemplate(ctx context.Context, id string) (string, string, err if err == nil { return string(name), string(data), nil } - if err != sql.ErrNoRows { err = errors.Wrap(err, "SELECT") logger.Error(err) } - - return "", "", nil + return "", "", err } // DeleteTemplate deletes a workflow template diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index c34766d42..4bfe65ac2 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -24,6 +24,11 @@ var state = map[int32]workflow.State{ 4: workflow.State_SUCCESS, } +const ( + errFailedToGetTemplate = "failed to get template with ID: %s" + errTemplateParsing = "failed to parse template with ID: %s" +) + // CreateWorkflow implements workflow.CreateWorkflow func (s *server) CreateWorkflow(ctx context.Context, in *workflow.CreateRequest) (*workflow.CreateResponse, error) { logger.Info("createworkflow") @@ -47,7 +52,6 @@ func (s *server) CreateWorkflow(ctx context.Context, in *workflow.CreateRequest) data, err := createYaml(ctx, s.db, in.Template, in.Hardware) if err != nil { metrics.CacheErrors.With(labels).Inc() - err = errors.Wrap(err, "failed to create Yaml") logger.Error(err) return &workflow.CreateResponse{}, err } @@ -269,22 +273,24 @@ func (s *server) ShowWorkflowEvents(req *workflow.GetRequest, stream workflow.Wo func createYaml(ctx context.Context, db db.Database, temp string, devices string) (string, error) { _, tempData, err := db.GetTemplate(ctx, temp) if err != nil { - return "", err + return "", errors.Wrapf(err, errFailedToGetTemplate, temp) } - return renderTemplate(string(tempData), []byte(devices)) + return renderTemplate(temp, tempData, []byte(devices)) } -func renderTemplate(tempData string, devices []byte) (string, error) { +func renderTemplate(templateID, tempData string, devices []byte) (string, error) { var hardware map[string]interface{} err := json.Unmarshal(devices, &hardware) if err != nil { + err = errors.Wrapf(err, errTemplateParsing, templateID) logger.Error(err) - return "", nil + return "", err } t := template.New("workflow-template") _, err = t.Parse(string(tempData)) if err != nil { + err = errors.Wrapf(err, errTemplateParsing, templateID) logger.Error(err) return "", nil } @@ -292,7 +298,9 @@ func renderTemplate(tempData string, devices []byte) (string, error) { buf := new(bytes.Buffer) err = t.Execute(buf, hardware) if err != nil { - return "", nil + err = errors.Wrapf(err, errTemplateParsing, templateID) + logger.Error(err) + return "", err } return buf.String(), nil } From a35bf293da793545b1c4c9b7674b9faa64255903 Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Thu, 10 Sep 2020 12:25:52 +0530 Subject: [PATCH 2/2] adding tests for CreateWorkflow Signed-off-by: Gaurav Gahlot --- db/mock/mock.go | 7 +++ db/mock/template.go | 2 +- db/mock/workflow.go | 2 +- grpc-server/workflow_test.go | 112 +++++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 grpc-server/workflow_test.go diff --git a/db/mock/mock.go b/db/mock/mock.go index 8773a5dfb..789efe88a 100644 --- a/db/mock/mock.go +++ b/db/mock/mock.go @@ -4,11 +4,15 @@ import ( "context" "time" + "github.com/google/uuid" + "github.com/tinkerbell/tink/db" pb "github.com/tinkerbell/tink/protos/workflow" ) // DB is the mocked implementation of Database interface type DB struct { + // workflow + CreateWorkflowFunc func(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error GetfromWfDataTableFunc func(ctx context.Context, req *pb.GetWorkflowDataRequest) ([]byte, error) InsertIntoWfDataTableFunc func(ctx context.Context, req *pb.UpdateWorkflowDataRequest) error GetWorkflowMetadataFunc func(ctx context.Context, req *pb.GetWorkflowDataRequest) ([]byte, error) @@ -18,4 +22,7 @@ type DB struct { GetWorkflowActionsFunc func(ctx context.Context, wfID string) (*pb.WorkflowActionList, error) UpdateWorkflowStateFunc func(ctx context.Context, wfContext *pb.WorkflowContext) error InsertIntoWorkflowEventTableFunc func(ctx context.Context, wfEvent *pb.WorkflowActionStatus, time time.Time) error + + // template + GetTemplateFunc func(ctx context.Context, id string) (string, string, error) } diff --git a/db/mock/template.go b/db/mock/template.go index 5031660a8..6e083c718 100644 --- a/db/mock/template.go +++ b/db/mock/template.go @@ -37,7 +37,7 @@ func (d DB) CreateTemplate(ctx context.Context, name string, data string, id uui // GetTemplate returns a workflow template func (d DB) GetTemplate(ctx context.Context, id string) (string, string, error) { - return "", "", nil + return d.GetTemplateFunc(ctx, id) } // DeleteTemplate deletes a workflow template diff --git a/db/mock/workflow.go b/db/mock/workflow.go index c3504692e..b49a875ce 100644 --- a/db/mock/workflow.go +++ b/db/mock/workflow.go @@ -11,7 +11,7 @@ import ( // CreateWorkflow creates a new workflow func (d DB) CreateWorkflow(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error { - return nil + return d.CreateWorkflowFunc(ctx, wf, data, id) } // InsertIntoWfDataTable : Insert ephemeral data in workflow_data table diff --git a/grpc-server/workflow_test.go b/grpc-server/workflow_test.go new file mode 100644 index 000000000..aa1420a83 --- /dev/null +++ b/grpc-server/workflow_test.go @@ -0,0 +1,112 @@ +package grpcserver + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/tinkerbell/tink/db" + "github.com/tinkerbell/tink/db/mock" + "github.com/tinkerbell/tink/protos/workflow" +) + +const ( + templateID = "e29b6444-1de7-4a69-bf25-6ea4ae869005" + hw = `{"device_1": "08:00:27:00:00:01"}` + templateData = `version: "0.1" +name: hello_world_workflow +global_timeout: 600 +tasks: + - name: "hello world" + worker: "{{.device_1}}" + actions: + - name: "hello_world" + image: hello-world + timeout: 60` +) + +func TestCreateWorkflow(t *testing.T) { + type ( + args struct { + db mock.DB + wfTemplate, wfHardware string + } + want struct { + expectedError bool + } + ) + testCases := map[string]struct { + args args + want want + }{ + "FailedToGetTempalte": { + args: args{ + db: mock.DB{ + GetTemplateFunc: func(ctx context.Context, id string) (string, string, error) { + return "", "", errors.New("failed to get template") + }, + }, + wfTemplate: templateID, + wfHardware: hw, + }, + want: want{ + expectedError: true, + }, + }, + "FailedCreatingWorkflow": { + args: args{ + db: mock.DB{ + GetTemplateFunc: func(ctx context.Context, id string) (string, string, error) { + return "", templateData, nil + }, + CreateWorkflowFunc: func(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error { + return errors.New("failed to create a workfow") + }, + }, + wfTemplate: templateID, + wfHardware: hw, + }, + want: want{ + expectedError: true, + }, + }, + "SuccessCreatingWorkflow": { + args: args{ + db: mock.DB{ + GetTemplateFunc: func(ctx context.Context, id string) (string, string, error) { + return "", templateData, nil + }, + CreateWorkflowFunc: func(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error { + return nil + }, + }, + wfTemplate: templateID, + wfHardware: hw, + }, + want: want{ + expectedError: false, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + s := testServer(tc.args.db) + res, err := s.CreateWorkflow(context.TODO(), &workflow.CreateRequest{ + Hardware: tc.args.wfHardware, + Template: tc.args.wfTemplate, + }) + if err != nil { + assert.Error(t, err) + assert.Empty(t, res) + assert.True(t, tc.want.expectedError) + return + } + assert.NoError(t, err) + assert.NotEmpty(t, res) + assert.False(t, tc.want.expectedError) + }) + } +}