Skip to content

Commit

Permalink
Avoid using context.TODO() in unit tests (#360)
Browse files Browse the repository at this point in the history 
Signed-off-by: Gaurav Gahlot <[email protected]>

## Description

It is not a good practice to use a never-ending context in unit tests. It is suggested that a unit test should always have a timeout. 

## How Has This Been Tested?

`make test`

## How are existing users impacted? What migration steps/scripts do we need?

No impact.
  • Loading branch information
@mergify
mergify[bot] authored Nov 9, 2020
2 parents 384fe30 + db26bdc commit 0e8e573
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 26 deletions.
8 changes: 6 additions & 2 deletions grpc-server/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,14 @@ func TestCreateTemplate(t *testing.T) {
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name := range testCases {
tc := testCases[name]
t.Run(name, func(t *testing.T) {
t.Parallel()
s := testServer(tc.args.db)
res, err := s.CreateTemplate(context.TODO(), &pb.WorkflowTemplate{Name: tc.args.name, Data: tc.args.template})
res, err := s.CreateTemplate(ctx, &pb.WorkflowTemplate{Name: tc.args.name, Data: tc.args.template})
if tc.want.expectedError {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -262,12 +264,14 @@ func TestGetTemplate(t *testing.T) {
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name := range testCases {
tc := testCases[name]
t.Run(name, func(t *testing.T) {
t.Parallel()
s := testServer(tc.args.db)
res, err := s.GetTemplate(context.TODO(), tc.args.getRequest)
res, err := s.GetTemplate(ctx, tc.args.getRequest)
if tc.err {
assert.Error(t, err)
} else {
Expand Down
63 changes: 40 additions & 23 deletions grpc-server/tinkerbell_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const (
invalidID = "d699-4e9f-a29c-a5890ccbd"
actionName = "install-rootfs"
taskName = "ubuntu-provisioning"

defaultTestTimeout = time.Millisecond * 10
)

var wfData = []byte("{'os': 'ubuntu', 'base_url': 'http://192.168.1.1/'}")
Expand Down Expand Up @@ -115,12 +117,13 @@ func TestGetWorkflowContextList(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
res, err := s.GetWorkflowContextList(
context.TODO(), &pb.WorkflowContextRequest{WorkerId: tc.args.workerID},
)
res, err := s.GetWorkflowContextList(ctx, &pb.WorkflowContextRequest{WorkerId: tc.args.workerID})
if err != nil {
assert.Error(t, err)
assert.Nil(t, res)
Expand Down Expand Up @@ -197,12 +200,13 @@ func TestGetWorkflowActions(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
res, err := s.GetWorkflowActions(
context.TODO(), &pb.WorkflowActionsRequest{WorkflowId: tc.args.workflowID},
)
res, err := s.GetWorkflowActions(ctx, &pb.WorkflowActionsRequest{WorkflowId: tc.args.workflowID})
if err != nil {
assert.True(t, tc.want.expectedError)
assert.Error(t, err)
Expand Down Expand Up @@ -537,10 +541,13 @@ func TestReportActionStatus(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
res, err := s.ReportActionStatus(context.TODO(),
res, err := s.ReportActionStatus(ctx,
&pb.WorkflowActionStatus{
WorkflowId: tc.args.workflowID,
ActionName: tc.args.actionName,
Expand Down Expand Up @@ -614,11 +621,14 @@ func TestUpdateWorkflowData(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
res, err := s.UpdateWorkflowData(
context.TODO(), &pb.UpdateWorkflowDataRequest{
ctx, &pb.UpdateWorkflowDataRequest{
WorkflowId: tc.args.workflowID,
Data: tc.args.data,
})
Expand Down Expand Up @@ -701,12 +711,13 @@ func TestGetWorkflowData(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
s := testServer(tc.args.db)
t.Run(name, func(t *testing.T) {
res, err := s.GetWorkflowData(
context.TODO(), &pb.GetWorkflowDataRequest{WorkflowId: tc.args.workflowID},
)
res, err := s.GetWorkflowData(ctx, &pb.GetWorkflowDataRequest{WorkflowId: tc.args.workflowID})
if err != nil {
assert.True(t, tc.want.expectedError)
assert.Error(t, err)
Expand Down Expand Up @@ -870,12 +881,13 @@ func TestGetWorkflowMetadata(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
res, err := s.GetWorkflowMetadata(
context.TODO(), &pb.GetWorkflowDataRequest{WorkflowId: tc.args.workflowID},
)
res, err := s.GetWorkflowMetadata(ctx, &pb.GetWorkflowDataRequest{WorkflowId: tc.args.workflowID})
if err != nil {
assert.True(t, tc.want.expectedError)
assert.Error(t, err)
Expand Down Expand Up @@ -940,12 +952,13 @@ func TestGetWorkflowDataVersion(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
res, err := s.GetWorkflowDataVersion(
context.TODO(), &pb.GetWorkflowDataRequest{WorkflowId: workflowID},
)
res, err := s.GetWorkflowDataVersion(ctx, &pb.GetWorkflowDataRequest{WorkflowId: workflowID})
assert.Equal(t, tc.want.version, res.Version)
if err != nil {
assert.True(t, tc.want.expectedError)
Expand Down Expand Up @@ -1152,13 +1165,14 @@ func TestIsApplicableToSend(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
wfContext, _ := s.db.GetWorkflowContexts(context.TODO(), workflowID)
res := isApplicableToSend(
context.TODO(), wfContext, workerID, s.db,
)
wfContext, _ := s.db.GetWorkflowContexts(ctx, workflowID)
res := isApplicableToSend(ctx, wfContext, workerID, s.db)
assert.Equal(t, tc.want.isApplicable, res)
})
}
Expand Down Expand Up @@ -1243,11 +1257,14 @@ func TestIsLastAction(t *testing.T) {
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
s := testServer(tc.args.db)
wfContext, _ := s.db.GetWorkflowContexts(context.TODO(), workflowID)
actions, _ := s.db.GetWorkflowActions(context.TODO(), workflowID)
wfContext, _ := s.db.GetWorkflowContexts(ctx, workflowID)
actions, _ := s.db.GetWorkflowActions(ctx, workflowID)
res := isLastAction(wfContext, actions)
assert.Equal(t, tc.want.isLastAction, res)
})
Expand Down
4 changes: 3 additions & 1 deletion grpc-server/workflow_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,12 @@ func TestCreateWorkflow(t *testing.T) {
},
}

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
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{
res, err := s.CreateWorkflow(ctx, &workflow.CreateRequest{
Hardware: tc.args.wfHardware,
Template: tc.args.wfTemplate,
})
Expand Down

0 comments on commit 0e8e573

Please sign in to comment.