Skip to content

Commit

Permalink
chore: apply reviewer feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Guilhem Barthés <[email protected]>
  • Loading branch information
guilhem-barthes committed Feb 12, 2024
1 parent b37cc5c commit e4677fb
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/service/computetaskstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ func (s *ComputeTaskService) onDone(e *fsm.Event) {
metrics.TaskUpdateCascadeSize.WithLabelValues(s.GetChannel(), string(transitionTodo)).Observe(float64(len(children)))
}

// startChildrenTaskFromParents propagates the DONE status of a parent to the task.
// This will iterate over task parents and mark it as TODO if all parents are DONE.
// startChildrenTaskFromParents checks which tasks can be started when a parent finishes.
// For each child task, it will check that the function finished building and the other parents statuses are all DONE.
func (s *ComputeTaskService) startChildrenTaskFromParents(triggeringParent, child *asset.ComputeTask) error {
logger := s.GetLogger().With().
Str("triggeringParent", triggeringParent.Key).
Expand Down Expand Up @@ -317,7 +317,7 @@ func (s *ComputeTaskService) onFailure(e *fsm.Event) {
}
}

// getInitialStatusFromParents will infer task status from its parents statuses.
// getInitialStatus will infer task status from its parents statuses.
func getInitialStatus(parents []*asset.ComputeTask, function *asset.Function) asset.ComputeTaskStatus {
var status asset.ComputeTaskStatus
var doneParents = 0
Expand Down
5 changes: 5 additions & 0 deletions lib/service/computetaskstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ func TestGetInitialStatus(t *testing.T) {
function: &asset.Function{Status: asset.FunctionStatus_FUNCTION_STATUS_READY},
outcome: asset.ComputeTaskStatus_STATUS_WAITING,
},
"parent waiting + function not ready": {
parents: []*asset.ComputeTask{{Status: asset.ComputeTaskStatus_STATUS_DONE}, {Status: asset.ComputeTaskStatus_STATUS_WAITING}},
function: &asset.Function{Status: asset.FunctionStatus_FUNCTION_STATUS_BUILDING},
outcome: asset.ComputeTaskStatus_STATUS_WAITING,
},
"parent ready + function not ready": {
parents: []*asset.ComputeTask{{Status: asset.ComputeTaskStatus_STATUS_DONE}, {Status: asset.ComputeTaskStatus_STATUS_DONE}},
function: &asset.Function{Status: asset.FunctionStatus_FUNCTION_STATUS_BUILDING},
Expand Down
69 changes: 69 additions & 0 deletions lib/service/functionstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,72 @@ func TestUpdateFunctionStateFailed(t *testing.T) {
dbal.AssertExpectations(t)
es.AssertExpectations(t)
}

func TestUpdateFunctionStateReady(t *testing.T) {
cases := map[string]struct {
parents []*asset.ComputeTask
becomeTodo bool
}{
"no parents": {
parents: []*asset.ComputeTask{},
becomeTodo: true,
},
"2 parents done": {
parents: []*asset.ComputeTask{{Status: asset.ComputeTaskStatus_STATUS_DONE}, {Status: asset.ComputeTaskStatus_STATUS_DONE}},
becomeTodo: true,
},
"1 parent done + 1 parent waiting": {
parents: []*asset.ComputeTask{{Status: asset.ComputeTaskStatus_STATUS_DONE}, {Status: asset.ComputeTaskStatus_STATUS_WAITING}},
becomeTodo: false,
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
dbal := new(persistence.MockDBAL)
ctdbal := new(persistence.MockDBAL)
es := new(MockEventAPI)
provider := newMockedProvider()

service := NewFunctionService(provider)

provider.On("GetFunctionDBAL").Return(dbal)
provider.On("GetComputeTaskService").Return(NewComputeTaskService(provider))
provider.On("GetEventService").Return(es)
provider.On("GetComputeTaskDBAL").Return(ctdbal)

functionKey := "uuid"
task := &asset.ComputeTask{
Key: "taskuuid",
Status: asset.ComputeTaskStatus_STATUS_WAITING,
Worker: "worker",
ComputePlanKey: "uuidcp",
}

dbal.On("GetFunction", "uuid").Return(&asset.Function{
Key: functionKey,
Status: asset.FunctionStatus_FUNCTION_STATUS_BUILDING,
Owner: "owner",
}, nil)

es.On("RegisterEvents", mock.Anything).Return(nil)

updatedFunction := &asset.Function{Key: functionKey, Status: asset.FunctionStatus_FUNCTION_STATUS_READY, Owner: "owner"}

dbal.On("UpdateFunction", updatedFunction).Return(nil)
ctdbal.On("GetFunctionFromTasksWithStatus", functionKey, []asset.ComputeTaskStatus{asset.ComputeTaskStatus_STATUS_WAITING}).Return([]*asset.ComputeTask{task}, nil)
ctdbal.On("GetComputeTaskParents", task.Key).Return(tc.parents, nil).Once()

if tc.becomeTodo {
ctdbal.On("UpdateComputeTaskStatus", task.Key, asset.ComputeTaskStatus_STATUS_TODO).Return(nil).Once()
}

err := service.ApplyFunctionAction("uuid", asset.FunctionAction_FUNCTION_ACTION_READY, "", "owner")
assert.NoError(t, err)

provider.AssertExpectations(t)
ctdbal.AssertExpectations(t)
dbal.AssertExpectations(t)
es.AssertExpectations(t)
})
}
}

0 comments on commit e4677fb

Please sign in to comment.