From 2b4a3ab137fbb3dd1b2f1bdba39ec5df039f42ef Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 3 Oct 2023 23:21:21 +0100 Subject: [PATCH] bugfix: broken unit tests --- internal/executor/executor.go | 21 ++++++++++++++------- internal/executor/executor_test.go | 8 +++++--- internal/grpcapi/grpcApi.go | 7 ++++++- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/internal/executor/executor.go b/internal/executor/executor.go index ae6bbab..9bb18cd 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -1,7 +1,6 @@ package executor import ( - pb "github.com/OliveTin/OliveTin/gen/grpc" acl "github.com/OliveTin/OliveTin/internal/acl" config "github.com/OliveTin/OliveTin/internal/config" log "github.com/sirupsen/logrus" @@ -13,6 +12,7 @@ import ( "os/exec" "runtime" "time" + "sync" ) // Executor represents a helper class for executing commands. It's main method @@ -99,11 +99,14 @@ func (e *Executor) AddListener(m listener) { } // ExecRequest processes an ExecutionRequest -func (e *Executor) ExecRequest(req *ExecutionRequest) *pb.StartActionResponse { +func (e *Executor) ExecRequest(req *ExecutionRequest) (*sync.WaitGroup, string) { + req.executor = e + // req.UUID is now set by the client, so that they can track the request // from start to finish. This means that a malicious client could send // duplicate UUIDs (or just random strings), but this is the only way. - req.executor = e + req.uuid = uuid.New().String() + req.logEntry = &InternalLogEntry{ DatetimeStarted: time.Now().Format("2006-01-02 15:04:05"), ActionTitle: req.ActionName, @@ -121,11 +124,15 @@ func (e *Executor) ExecRequest(req *ExecutionRequest) *pb.StartActionResponse { listener.OnExecutionStarted(req.ActionName) } - go e.execChain(req) + wg := new(sync.WaitGroup) + wg.Add(1); - return &pb.StartActionResponse{ - ExecutionUuid: req.UUID, - } + go func() { + e.execChain(req) + defer wg.Done(); + }() + + return wg, req.uuid; } func (e *Executor) execChain(req *ExecutionRequest) { diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index e850d79..9296882 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -42,11 +42,12 @@ func TestCreateExecutorAndExec(t *testing.T) { }, } - e.ExecRequest(&req) assert.NotNil(t, e, "Create an executor") - assert.NotNil(t, e.ExecRequest(&req), "Execute a request") + wg, _ := e.ExecRequest(&req) + wg.Wait() + assert.Equal(t, int32(0), req.logEntry.ExitCode, "Exit code is zero") } @@ -59,7 +60,8 @@ func TestExecNonExistant(t *testing.T) { Cfg: cfg, } - e.ExecRequest(&req) + wg, _ := e.ExecRequest(&req) + wg.Wait() assert.Equal(t, int32(-1337), req.logEntry.ExitCode, "Log entry is set to an internal error code") assert.Equal(t, "", req.logEntry.ActionIcon, "Log entry icon wasnt found") diff --git a/internal/grpcapi/grpcApi.go b/internal/grpcapi/grpcApi.go index 431333c..acfe172 100644 --- a/internal/grpcapi/grpcApi.go +++ b/internal/grpcapi/grpcApi.go @@ -42,7 +42,12 @@ func (api *oliveTinAPI) StartAction(ctx ctx.Context, req *pb.StartActionRequest) Cfg: cfg, } - return api.executor.ExecRequest(&execReq), nil + _, uuid := api.executor.ExecRequest(&execReq); + + return &pb.StartActionResponse { + ExecutionUuid: uuid, + }, nil; + } func (api *oliveTinAPI) ExecutionStatus(ctx ctx.Context, req *pb.ExecutionStatusRequest) (*pb.ExecutionStatusResponse, error) {