Implement linting issues and merge request comments

.gitignore

@@ -3,6 +3,7 @@ poseidon
 
 # Configuration file
 configuration.yaml
+tests/e2e/configuration.yaml
 
 # TLS certificate/key
 *.crt

api/environments_test.go

@@ -42,7 +42,7 @@ func TestCreateOrUpdateEnvironmentTestSuite(t *testing.T) {
 
 func (s *CreateOrUpdateEnvironmentTestSuite) SetupTest() {
 	s.EnvironmentControllerTestSuite.SetupTest()
-	s.id = tests.DefaultEnvironmentIdAsString
+	s.id = tests.DefaultEnvironmentIDAsString
 	testURL, err := s.router.Get(createOrUpdateRouteName).URL(executionEnvironmentIDKey, s.id)
 	if err != nil {
 		s.T().Fatal(err)
@@ -71,7 +71,7 @@ func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsBadRequestWhenBadBody()
 }
 
 func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsInternalServerErrorWhenManagerReturnsError() {
-	testError := tests.DefaultError
+	testError := tests.ErrDefault
 	s.manager.
 		On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest")).
 		Return(false, testError)

api/runners.go

@@ -45,7 +45,7 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req
 	if err := parseJSONRequestBody(writer, request, runnerRequest); err != nil {
 		return
 	}
-	environmentId := runner.EnvironmentId(runnerRequest.ExecutionEnvironmentId)
+	environmentId := runner.EnvironmentID(runnerRequest.ExecutionEnvironmentId)
 	nextRunner, err := r.manager.Claim(environmentId)
 	if err != nil {
 		if err == runner.ErrUnknownExecutionEnvironment {

api/runners_test.go

@@ -173,13 +173,13 @@ type UpdateFileSystemRouteTestSuite struct {
 
 func (s *UpdateFileSystemRouteTestSuite) SetupTest() {
 	s.RunnerRouteTestSuite.SetupTest()
-	routeUrl, err := s.router.Get(UpdateFileSystemPath).URL(RunnerIdKey, tests.DefaultMockId)
+	routeUrl, err := s.router.Get(UpdateFileSystemPath).URL(RunnerIdKey, tests.DefaultMockID)
 	if err != nil {
 		s.T().Fatal(err)
 	}
 	s.path = routeUrl.String()
 	s.runnerMock = &runner.RunnerMock{}
-	s.runnerManager.On("Get", tests.DefaultMockId).Return(s.runnerMock, nil)
+	s.runnerManager.On("Get", tests.DefaultMockID).Return(s.runnerMock, nil)
 	s.recorder = httptest.NewRecorder()
 }
 

environment/manager.go

@@ -28,7 +28,7 @@ type Manager interface {
 	Delete(id string)
 }
 
-func NewNomadEnvironmentManager(runnerManager runner.Manager, apiClient nomad.ExecutorApi) *NomadEnvironmentManager {
+func NewNomadEnvironmentManager(runnerManager runner.Manager, apiClient nomad.ExecutorAPI) *NomadEnvironmentManager {
 	environmentManager := &NomadEnvironmentManager{runnerManager, apiClient, *parseJob(defaultJobHCL)}
 	environmentManager.Load()
 	return environmentManager
@@ -36,7 +36,7 @@ func NewNomadEnvironmentManager(runnerManager runner.Manager, apiClient nomad.Ex
 
 type NomadEnvironmentManager struct {
 	runnerManager runner.Manager
-	api           nomad.ExecutorApi
+	api           nomad.ExecutorAPI
 	defaultJob    nomadApi.Job
 }
 
@@ -48,7 +48,7 @@ func (m *NomadEnvironmentManager) CreateOrUpdate(
 	if err != nil {
 		return false, err
 	}
-	exists := m.runnerManager.EnvironmentExists(runner.EnvironmentId(idInt))
+	exists := m.runnerManager.EnvironmentExists(runner.EnvironmentID(idInt))
 
 	err = m.registerJob(id,
 		request.PrewarmingPoolSize, request.CPULimit, request.MemoryLimit,
@@ -57,7 +57,7 @@ func (m *NomadEnvironmentManager) CreateOrUpdate(
 	if err == nil {
 		if !exists {
 			m.runnerManager.RegisterEnvironment(
-				runner.EnvironmentId(idInt), runner.NomadJobId(id), request.PrewarmingPoolSize)
+				runner.EnvironmentID(idInt), runner.NomadJobID(id), request.PrewarmingPoolSize)
 		}
 		return !exists, nil
 	}
@@ -70,5 +70,5 @@ func (m *NomadEnvironmentManager) Delete(id string) {
 
 func (m *NomadEnvironmentManager) Load() {
 	// ToDo: remove create default execution environment for debugging purposes
-	m.runnerManager.RegisterEnvironment(runner.EnvironmentId(0), "python", 5)
+	m.runnerManager.RegisterEnvironment(runner.EnvironmentID(0), "python", 5)
 }

environment/manager_test.go

@@ -48,17 +48,17 @@ func (s *CreateOrUpdateTestSuite) SetupTest() {
 }
 
 func (s *CreateOrUpdateTestSuite) mockEnvironmentExists(exists bool) {
-	s.runnerManagerMock.On("EnvironmentExists", mock.AnythingOfType("EnvironmentId")).Return(exists)
+	s.runnerManagerMock.On("EnvironmentExists", mock.AnythingOfType("EnvironmentID")).Return(exists)
 }
 
 func (s *CreateOrUpdateTestSuite) mockRegisterEnvironment() *mock.Call {
 	return s.runnerManagerMock.On("RegisterEnvironment",
-		mock.AnythingOfType("EnvironmentId"), mock.AnythingOfType("NomadJobId"), mock.AnythingOfType("uint")).
+		mock.AnythingOfType("EnvironmentID"), mock.AnythingOfType("NomadJobID"), mock.AnythingOfType("uint")).
 		Return()
 }
 
 func (s *CreateOrUpdateTestSuite) createJobForRequest() *nomadApi.Job {
-	return createJob(s.manager.defaultJob, tests.DefaultEnvironmentIdAsString,
+	return createJob(s.manager.defaultJob, tests.DefaultEnvironmentIDAsString,
 		s.request.PrewarmingPoolSize, s.request.CPULimit, s.request.MemoryLimit,
 		s.request.Image, s.request.NetworkAccess, s.request.ExposedPorts)
 }
@@ -78,7 +78,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsRegistersCorrectJob()
 	s.mockEnvironmentExists(true)
 	expectedJob := s.createJobForRequest()
 
-	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request)
+	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
 	s.NoError(err)
 	s.False(created)
 	s.apiMock.AssertCalled(s.T(), "RegisterNomadJob", expectedJob)
@@ -87,16 +87,16 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsRegistersCorrectJob()
 func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsOccurredErrorIsPassed() {
 	s.mockEnvironmentExists(true)
 
-	s.registerNomadJobMockCall.Return("", tests.DefaultError)
+	s.registerNomadJobMockCall.Return("", tests.ErrDefault)
-	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request)
+	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
 	s.False(created)
-	s.Equal(tests.DefaultError, err)
+	s.Equal(tests.ErrDefault, err)
 }
 
 func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentExistsReturnsFalse() {
 	s.mockEnvironmentExists(true)
 
-	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request)
+	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
 	s.NoError(err)
 	s.False(created)
 }
@@ -107,7 +107,7 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrec
 
 	expectedJob := s.createJobForRequest()
 
-	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request)
+	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
 	s.NoError(err)
 	s.True(created)
 	s.apiMock.AssertCalled(s.T(), "RegisterNomadJob", expectedJob)
@@ -117,20 +117,22 @@ func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistRegistersCorrec
 	s.mockEnvironmentExists(false)
 	s.mockRegisterEnvironment()
 
-	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request)
+	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
 	s.True(created)
 	s.NoError(err)
 	s.runnerManagerMock.AssertCalled(s.T(), "RegisterEnvironment",
-		runner.EnvironmentId(tests.DefaultEnvironmentIdAsInteger), runner.NomadJobId(tests.DefaultEnvironmentIdAsString), s.request.PrewarmingPoolSize)
+		runner.EnvironmentID(tests.DefaultEnvironmentIDAsInteger),
+		runner.NomadJobID(tests.DefaultEnvironmentIDAsString),
+		s.request.PrewarmingPoolSize)
 }
 
 func (s *CreateOrUpdateTestSuite) TestWhenEnvironmentDoesNotExistOccurredErrorIsPassedAndNoEnvironmentRegistered() {
 	s.mockEnvironmentExists(false)
 	s.mockRegisterEnvironment()
 
-	s.registerNomadJobMockCall.Return("", tests.DefaultError)
+	s.registerNomadJobMockCall.Return("", tests.ErrDefault)
-	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIdAsString, s.request)
+	created, err := s.manager.CreateOrUpdate(tests.DefaultEnvironmentIDAsString, s.request)
 	s.False(created)
-	s.Equal(tests.DefaultError, err)
+	s.Equal(tests.ErrDefault, err)
 	s.runnerManagerMock.AssertNotCalled(s.T(), "RegisterEnvironment")
 }

nomad/api_querier.go

@@ -43,14 +43,14 @@ type apiQuerier interface {
 	AllocationStream(ctx context.Context) (<-chan *nomadApi.Events, error)
 }
 
-// nomadApiClient implements the nomadApiQuerier interface and provides access to a real Nomad API.
+// nomadAPIClient implements the nomadApiQuerier interface and provides access to a real Nomad API.
-type nomadApiClient struct {
+type nomadAPIClient struct {
 	client       *nomadApi.Client
 	namespace    string
 	queryOptions *nomadApi.QueryOptions
 }
 
-func (nc *nomadApiClient) init(nomadURL *url.URL, nomadNamespace string) (err error) {
+func (nc *nomadAPIClient) init(nomadURL *url.URL, nomadNamespace string) (err error) {
 	nc.client, err = nomadApi.NewClient(&nomadApi.Config{
 		Address:   nomadURL.String(),
 		TLSConfig: &nomadApi.TLSConfig{},
@@ -63,8 +63,8 @@ func (nc *nomadApiClient) init(nomadURL *url.URL, nomadNamespace string) (err er
 	return err
 }
 
-func (nc *nomadApiClient) DeleteRunner(runnerId string) (err error) {
+func (nc *nomadAPIClient) DeleteRunner(runnerID string) (err error) {
-	allocation, _, err := nc.client.Allocations().Info(runnerId, nc.queryOptions)
+	allocation, _, err := nc.client.Allocations().Info(runnerID, nc.queryOptions)
 	if err != nil {
 		return
 	}
@@ -72,7 +72,7 @@ func (nc *nomadApiClient) DeleteRunner(runnerId string) (err error) {
 	return err
 }
 
-func (nc *nomadApiClient) ExecuteCommand(allocationID string,
+func (nc *nomadAPIClient) ExecuteCommand(allocationID string,
 	ctx context.Context, command []string, tty bool,
 	stdin io.Reader, stdout, stderr io.Writer) (int, error) {
 	allocation, _, err := nc.client.Allocations().Info(allocationID, nil)
@@ -82,12 +82,12 @@ func (nc *nomadApiClient) ExecuteCommand(allocationID string,
 	return nc.client.Allocations().Exec(ctx, allocation, TaskName, tty, command, stdin, stdout, stderr, nil, nil)
 }
 
-func (nc *nomadApiClient) loadRunners(jobId string) (allocationListStub []*nomadApi.AllocationListStub, err error) {
+func (nc *nomadAPIClient) loadRunners(jobID string) (allocationListStub []*nomadApi.AllocationListStub, err error) {
-	allocationListStub, _, err = nc.client.Jobs().Allocations(jobId, true, nc.queryOptions)
+	allocationListStub, _, err = nc.client.Jobs().Allocations(jobID, true, nc.queryOptions)
 	return
 }
 
-func (nc *nomadApiClient) RegisterNomadJob(job *nomadApi.Job) (string, error) {
+func (nc *nomadAPIClient) RegisterNomadJob(job *nomadApi.Job) (string, error) {
 	job.Namespace = &nc.namespace
 	resp, _, err := nc.client.Jobs().Register(job, nil)
 	if err != nil {
@@ -102,7 +102,7 @@ func (nc *nomadApiClient) RegisterNomadJob(job *nomadApi.Job) (string, error) {
 	return resp.EvalID, nil
 }
 
-func (nc *nomadApiClient) EvaluationStream(evalID string, ctx context.Context) (stream <-chan *nomadApi.Events, err error) {
+func (nc *nomadAPIClient) EvaluationStream(evalID string, ctx context.Context) (stream <-chan *nomadApi.Events, err error) {
 	stream, err = nc.client.EventStream().Stream(
 		ctx,
 		map[nomadApi.Topic][]string{
@@ -113,7 +113,7 @@ func (nc *nomadApiClient) EvaluationStream(evalID string, ctx context.Context) (
 	return
 }
 
-func (nc *nomadApiClient) AllocationStream(ctx context.Context) (stream <-chan *nomadApi.Events, err error) {
+func (nc *nomadAPIClient) AllocationStream(ctx context.Context) (stream <-chan *nomadApi.Events, err error) {
 	stream, err = nc.client.EventStream().Stream(
 		ctx,
 		map[nomadApi.Topic][]string{

nomad/executor_api_mock.go

@@ -14,7 +14,7 @@ import (
 	url "net/url"
 )
 
-// ExecutorApiMock is an autogenerated mock type for the ExecutorApi type
+// ExecutorApiMock is an autogenerated mock type for the ExecutorAPI type
 type ExecutorApiMock struct {
 	mock.Mock
 }

nomad/job.go

@@ -11,25 +11,25 @@ const (
 )
 
 // LoadJobList loads the list of jobs from the Nomad api.
-func (nc *nomadApiClient) LoadJobList() (list []*nomadApi.JobListStub, err error) {
+func (nc *nomadAPIClient) LoadJobList() (list []*nomadApi.JobListStub, err error) {
 	list, _, err = nc.client.Jobs().List(nc.queryOptions)
 	return
 }
 
 // JobScale returns the scale of the passed job.
-func (nc *nomadApiClient) JobScale(jobId string) (jobScale uint, err error) {
+func (nc *nomadAPIClient) JobScale(jobID string) (jobScale uint, err error) {
-	status, _, err := nc.client.Jobs().ScaleStatus(jobId, nc.queryOptions)
+	status, _, err := nc.client.Jobs().ScaleStatus(jobID, nc.queryOptions)
 	if err != nil {
 		return
 	}
 	// ToDo: Consider counting also the placed and desired allocations
-	jobScale = uint(status.TaskGroups[fmt.Sprintf(TaskGroupNameFormat, jobId)].Running)
+	jobScale = uint(status.TaskGroups[fmt.Sprintf(TaskGroupNameFormat, jobID)].Running)
 	return
 }
 
 // SetJobScale sets the scaling count of the passed job to Nomad.
-func (nc *nomadApiClient) SetJobScale(jobId string, count uint, reason string) (err error) {
+func (nc *nomadAPIClient) SetJobScale(jobID string, count uint, reason string) (err error) {
 	intCount := int(count)
-	_, _, err = nc.client.Jobs().Scale(jobId, fmt.Sprintf(TaskGroupNameFormat, jobId), &intCount, reason, false, nil, nil)
+	_, _, err = nc.client.Jobs().Scale(jobID, fmt.Sprintf(TaskGroupNameFormat, jobID), &intCount, reason, false, nil, nil)
 	return
 }

nomad/nomad.go

@@ -14,50 +14,47 @@ import (
 var (
 	log                              = logging.GetLogger("nomad")
 	ErrorExecutorCommunicationFailed = errors.New("communication with executor failed")
-	errEvaluation         = errors.New("evaluation could not complete")
+	errEvaluation                    = errors.New("evaluation could not complete")
-	errPlacingAllocations = errors.New("failed to place all allocations")
+	errPlacingAllocations            = errors.New("failed to place all allocations")
 )
 
 type AllocationProcessor func(*nomadApi.Allocation)
 
-// ExecutorApi provides access to an container orchestration solution
+// ExecutorAPI provides access to an container orchestration solution.
-type ExecutorApi interface {
+type ExecutorAPI interface {
 	apiQuerier
 
 	// LoadRunners loads all allocations of the specified job which are running and not about to get stopped.
 	LoadRunners(jobID string) (runnerIds []string, err error)
 
 	// MonitorEvaluation monitors the given evaluation ID.
-	// It waits until the evaluation reaches one of the states complete, cancelled or failed.
+	// It waits until the evaluation reaches one of the states complete, canceled or failed.
 	// If the evaluation was not successful, an error containing the failures is returned.
 	// See also https://github.com/hashicorp/nomad/blob/7d5a9ecde95c18da94c9b6ace2565afbfdd6a40d/command/monitor.go#L175
-	MonitorEvaluation(evalID string, ctx context.Context) error
+	MonitorEvaluation(evaluationID string, ctx context.Context) error
 
 	// WatchAllocations listens on the Nomad event stream for allocation events.
 	// Depending on the incoming event, any of the given function is executed.
 	WatchAllocations(ctx context.Context, onNewAllocation, onDeletedAllocation AllocationProcessor) error
 }
 
-// APIClient implements the ExecutorApi interface and can be used to perform different operations on the real Executor API and its return values.
+// APIClient implements the ExecutorAPI interface and can be used to perform different operations on the real
+// Executor API and its return values.
 type APIClient struct {
 	apiQuerier
 }
 
 // NewExecutorAPI creates a new api client.
 // One client is usually sufficient for the complete runtime of the API.
-func NewExecutorAPI(nomadURL *url.URL, nomadNamespace string) (ExecutorApi, error) {
+func NewExecutorAPI(nomadURL *url.URL, nomadNamespace string) (ExecutorAPI, error) {
-	client := &APIClient{apiQuerier: &nomadApiClient{}}
+	client := &APIClient{apiQuerier: &nomadAPIClient{}}
 	err := client.init(nomadURL, nomadNamespace)
 	return client, err
 }
 
 // init prepares an apiClient to be able to communicate to a provided Nomad API.
-func (a *APIClient) init(nomadURL *url.URL, nomadNamespace string) (err error) {
+func (a *APIClient) init(nomadURL *url.URL, nomadNamespace string) error {
-	err = a.apiQuerier.init(nomadURL, nomadNamespace)
+	return a.apiQuerier.init(nomadURL, nomadNamespace)
-	if err != nil {
-		return err
-	}
-	return nil
 }
 
 // LoadRunners loads the allocations of the specified job.
@@ -72,11 +69,11 @@ func (a *APIClient) LoadRunners(jobID string) (runnerIds []string, err error) {
 			runnerIds = append(runnerIds, stub.ID)
 		}
 	}
-	return
+	return runnerIds, nil
 }
 
-func (a *APIClient) MonitorEvaluation(evalID string, ctx context.Context) error {
+func (a *APIClient) MonitorEvaluation(evaluationID string, ctx context.Context) error {
-	stream, err := a.EvaluationStream(evalID, ctx)
+	stream, err := a.EvaluationStream(evaluationID, ctx)
 	if err != nil {
 		return fmt.Errorf("failed retrieving evaluation stream: %w", err)
 	}
@@ -120,8 +117,8 @@ func receiveAndHandleNomadAPIEvents(stream <-chan *nomadApi.Events, handler noma
 		}
 		for _, event := range events.Events {
 			// Don't take the address of the loop variable as the underlying value might change
-			localEvent := event
+			eventCopy := event
-			done, err := handler(&localEvent)
+			done, err := handler(&eventCopy)
 			if err != nil || done {
 				return err
 			}
@@ -130,11 +127,12 @@ func receiveAndHandleNomadAPIEvents(stream <-chan *nomadApi.Events, handler noma
 	return nil
 }
 
-// handleEvaluationEvent is a nomadAPIEventHandler that returns the status of an evaluation in the event.
+// handleEvaluationEvent is a nomadAPIEventHandler that returns whether the evaluation described by the event
+// was successful.
 func handleEvaluationEvent(event *nomadApi.Event) (bool, error) {
 	eval, err := event.Evaluation()
 	if err != nil {
-		return true, fmt.Errorf("failed monitoring evaluation: %w", err)
+		return true, fmt.Errorf("failed to monitor evaluation: %w", err)
 	}
 	switch eval.Status {
 	case structs.EvalStatusComplete, structs.EvalStatusCancelled, structs.EvalStatusFailed:
@@ -149,11 +147,13 @@ func handleEvaluationEvent(event *nomadApi.Event) (bool, error) {
 // map the state is persisted between multiple calls of this function.
 func handleAllocationEvent(startTime int64, pendingAllocations map[string]bool, event *nomadApi.Event,
 	onNewAllocation, onDeletedAllocation AllocationProcessor) error {
+	if event.Type != structs.TypeAllocationUpdated {
+		return nil
+	}
 	alloc, err := event.Allocation()
 	if err != nil {
-		return fmt.Errorf("failed retrieving allocation from event: %w", err)
+		return fmt.Errorf("failed to retrieve allocation from event: %w", err)
-	}
+	} else if alloc == nil {
-	if alloc == nil || event.Type != structs.TypeAllocationUpdated {
 		return nil
 	}
 
@@ -169,7 +169,7 @@ func handleAllocationEvent(startTime int64, pendingAllocations map[string]bool,
 		case structs.AllocDesiredStatusStop:
 			onDeletedAllocation(alloc)
 		case structs.AllocDesiredStatusRun:
-			// first event that marks the transition between pending and running
+			// is first event that marks the transition between pending and running?
 			_, ok := pendingAllocations[alloc.ID]
 			if ok {
 				onNewAllocation(alloc)
@@ -194,8 +194,8 @@ func checkEvaluation(eval *nomadApi.Evaluation) (err error) {
 		}
 	} else {
 		err = fmt.Errorf("evaluation %q finished with status %q but %w", eval.ID, eval.Status, errPlacingAllocations)
-		for tg, metrics := range eval.FailedTGAllocs {
+		for taskGroup, metrics := range eval.FailedTGAllocs {
-			err = fmt.Errorf("%w\n%s: %#v", err, tg, metrics)
+			err = fmt.Errorf("%w\n%s: %#v", err, taskGroup, metrics)
 		}
 		if eval.BlockedEval != "" {
 			err = fmt.Errorf("%w\nEvaluation %q waiting for additional capacity to place remainder", err, eval.BlockedEval)

nomad/nomad_test.go

@@ -132,13 +132,13 @@ var (
 const TestNamespace = "unit-tests"
 
 func TestApiClient_init(t *testing.T) {
-	client := &APIClient{apiQuerier: &nomadApiClient{}}
+	client := &APIClient{apiQuerier: &nomadAPIClient{}}
 	err := client.init(&TestURL, TestNamespace)
 	require.Nil(t, err)
 }
 
 func TestApiClientCanNotBeInitializedWithInvalidUrl(t *testing.T) {
-	client := &APIClient{apiQuerier: &nomadApiClient{}}
+	client := &APIClient{apiQuerier: &nomadAPIClient{}}
 	err := client.init(&url.URL{
 		Scheme: "http",
 		Host:   "http://127.0.0.1:4646",
@@ -147,7 +147,7 @@ func TestApiClientCanNotBeInitializedWithInvalidUrl(t *testing.T) {
 }
 
 func TestNewExecutorApiCanBeCreatedWithoutError(t *testing.T) {
-	expectedClient := &APIClient{apiQuerier: &nomadApiClient{}}
+	expectedClient := &APIClient{apiQuerier: &nomadAPIClient{}}
 	err := expectedClient.init(&TestURL, TestNamespace)
 	require.Nil(t, err)
 
@@ -222,19 +222,27 @@ func eventForEvaluation(t *testing.T, eval nomadApi.Evaluation) nomadApi.Event {
 
 // simulateNomadEventStream streams the given events sequentially to the stream channel.
 // It returns how many events have been processed until an error occurred.
-func simulateNomadEventStream(stream chan *nomadApi.Events, errChan chan error, events []*nomadApi.Events) (int, error) {
+func simulateNomadEventStream(
+	stream chan *nomadApi.Events,
+	errChan chan error,
+	events []*nomadApi.Events,
+) (int, error) {
 	eventsProcessed := 0
 	var e *nomadApi.Events
 	for _, e = range events {
 		select {
 		case err := <-errChan:
-			close(stream)
 			return eventsProcessed, err
 		case stream <- e:
 			eventsProcessed++
 		}
 	}
-	err := <-errChan
+	// Wait for last event being processed
+	var err error
+	select {
+	case <-time.After(10 * time.Millisecond):
+	case err = <-errChan:
+	}
 	return eventsProcessed, err
 }
 
@@ -298,7 +306,7 @@ func TestApiClient_MonitorEvaluationWithFailingEvent(t *testing.T) {
 	multipleEventsWithPending := nomadApi.Events{Events: []nomadApi.Event{
 		eventForEvaluation(t, pendingEval), eventForEvaluation(t, eval),
 	}}
-	eventsWithErr := nomadApi.Events{Err: tests.DefaultError, Events: []nomadApi.Event{{}}}
+	eventsWithErr := nomadApi.Events{Err: tests.ErrDefault, Events: []nomadApi.Event{{}}}
 
 	var cases = []struct {
 		streamedEvents          []*nomadApi.Events
@@ -316,7 +324,7 @@ func TestApiClient_MonitorEvaluationWithFailingEvent(t *testing.T) {
 			"it skips pending evaluation and fail"},
 		{[]*nomadApi.Events{&multipleEventsWithPending}, 1, evalErr,
 			"it handles multiple events per received event and fails"},
-		{[]*nomadApi.Events{&eventsWithErr}, 1, tests.DefaultError,
+		{[]*nomadApi.Events{&eventsWithErr}, 1, tests.ErrDefault,
 			"it fails with event error when event has error"},
 	}
 
@@ -498,17 +506,19 @@ func TestHandleAllocationEventBuffersPendingAllocation(t *testing.T) {
 
 func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationStreamCannotBeRetrieved(t *testing.T) {
 	apiMock := &apiQuerierMock{}
-	apiMock.On("AllocationStream", mock.Anything).Return(nil, tests.DefaultError)
+	apiMock.On("AllocationStream", mock.Anything).Return(nil, tests.ErrDefault)
 	apiClient := &APIClient{apiMock}
 
 	noop := func(a *nomadApi.Allocation) {}
 	err := apiClient.WatchAllocations(context.Background(), noop, noop)
-	assert.ErrorIs(t, err, tests.DefaultError)
+	assert.ErrorIs(t, err, tests.ErrDefault)
 }
 
-func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationCannotBeRetrievedWithoutReceivingFurtherEvents(t *testing.T) {
+func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationCannotBeRetrievedWithoutReceivingFurtherEvents(
+	t *testing.T) {
 	noop := func(a *nomadApi.Allocation) {}
 	event := nomadApi.Event{
+		Type:  structs.TypeAllocationUpdated,
 		Topic: nomadApi.TopicAllocation,
 		// This should fail decoding, as Allocation.ID is expected to be a string, not int
 		Payload: map[string]interface{}{"Allocation": map[string]interface{}{"ID": 1}},
@@ -517,7 +527,7 @@ func TestAPIClient_WatchAllocationsReturnsErrorWhenAllocationCannotBeRetrievedWi
 	require.Error(t, err)
 
 	events := []*nomadApi.Events{{Events: []nomadApi.Event{event}}, {}}
-	eventsProcessed, err := runAllocationWatching(t, events, noop, noop, context.Background())
+	eventsProcessed, err := runAllocationWatching(t, events, noop, noop)
 	assert.Error(t, err)
 	assert.Equal(t, 1, eventsProcessed)
 }
@@ -535,10 +545,7 @@ func assertWatchAllocation(t *testing.T, events []*nomadApi.Events,
 		deletedAllocations = append(deletedAllocations, alloc)
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
+	eventsProcessed, err := runAllocationWatching(t, events, onNewAllocation, onDeletedAllocation)
-	defer cancel()
-
-	eventsProcessed, err := runAllocationWatching(t, events, onNewAllocation, onDeletedAllocation, ctx)
 	assert.NoError(t, err)
 	assert.Equal(t, len(events), eventsProcessed)
 
@@ -550,13 +557,9 @@ func assertWatchAllocation(t *testing.T, events []*nomadApi.Events,
 // to the MonitorEvaluation method. It starts the MonitorEvaluation function as a goroutine
 // and sequentially transfers the events from the given array to a channel simulating the stream.
 func runAllocationWatching(t *testing.T, events []*nomadApi.Events,
-	onNewAllocation, onDeletedAllocation AllocationProcessor, ctx context.Context) (eventsProcessed int, err error) {
+	onNewAllocation, onDeletedAllocation AllocationProcessor) (eventsProcessed int, err error) {
 	t.Helper()
 	stream := make(chan *nomadApi.Events)
-	go func() {
-		<-ctx.Done()
-		close(stream)
-	}()
 	errChan := asynchronouslyWatchAllocations(stream, onNewAllocation, onDeletedAllocation)
 	return simulateNomadEventStream(stream, errChan, events)
 }
@@ -591,7 +594,7 @@ func eventForAllocation(t *testing.T, alloc *nomadApi.Allocation) nomadApi.Event
 
 	err := mapstructure.Decode(eventPayload{Allocation: alloc}, &payload)
 	if err != nil {
-		t.Fatalf("Couldn't encode allocation %v", err)
+		t.Fatalf("Couldn't decode allocation %v into payload map", err)
 		return nomadApi.Event{}
 	}
 	event := nomadApi.Event{
@@ -604,7 +607,7 @@ func eventForAllocation(t *testing.T, alloc *nomadApi.Allocation) nomadApi.Event
 
 func createAllocation(modifyTime int64, clientStatus, desiredStatus string) *nomadApi.Allocation {
 	return &nomadApi.Allocation{
-		ID:            tests.AllocationID,
+		ID:            tests.DefaultRunnerID,
 		ModifyTime:    modifyTime,
 		ClientStatus:  clientStatus,
 		DesiredStatus: desiredStatus,

runner/constants_test.go

@@ -3,6 +3,6 @@ package runner
 import "gitlab.hpi.de/codeocean/codemoon/poseidon/tests"
 
 const (
-	defaultEnvironmentId = EnvironmentId(tests.DefaultEnvironmentIdAsInteger)
+	defaultEnvironmentID = EnvironmentID(tests.DefaultEnvironmentIDAsInteger)
-	anotherEnvironmentId = EnvironmentId(tests.AnotherEnvironmentIdAsInteger)
+	anotherEnvironmentID = EnvironmentID(tests.AnotherEnvironmentIDAsInteger)
 )

runner/manager.go

@@ -17,29 +17,30 @@ var (
 	ErrRunnerNotFound              = errors.New("no runner found with this id")
 )
 
-type EnvironmentId int
+type EnvironmentID int
 
-func (e EnvironmentId) toString() string {
+func (e EnvironmentID) toString() string {
 	return string(rune(e))
 }
 
-type NomadJobId string
+type NomadJobID string
 
-// Manager keeps track of the used and unused runners of all execution environments in order to provide unused runners to new clients and ensure no runner is used twice.
+// Manager keeps track of the used and unused runners of all execution environments in order to provide unused
+// runners to new clients and ensure no runner is used twice.
 type Manager interface {
 	// RegisterEnvironment adds a new environment that should be managed.
-	RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint)
+	RegisterEnvironment(id EnvironmentID, nomadJobID NomadJobID, desiredIdleRunnersCount uint)
 
 	// EnvironmentExists returns whether the environment with the given id exists.
-	EnvironmentExists(id EnvironmentId) bool
+	EnvironmentExists(id EnvironmentID) bool
 
 	// Claim returns a new runner.
 	// It makes sure that the runner is not in use yet and returns an error if no runner could be provided.
-	Claim(id EnvironmentId) (Runner, error)
+	Claim(id EnvironmentID) (Runner, error)
 
 	// Get returns the used runner with the given runnerId.
 	// If no runner with the given runnerId is currently used, it returns an error.
-	Get(runnerId string) (Runner, error)
+	Get(runnerID string) (Runner, error)
 
 	// Return signals that the runner is no longer used by the caller and can be claimed by someone else.
 	// The runner is deleted or cleaned up for reuse depending on the used executor.
@@ -47,12 +48,15 @@ type Manager interface {
 }
 
 type NomadRunnerManager struct {
-	apiClient   nomad.ExecutorApi
+	apiClient   nomad.ExecutorAPI
 	jobs        NomadJobStorage
 	usedRunners Storage
 }
 
-func NewNomadRunnerManager(apiClient nomad.ExecutorApi, ctx context.Context) *NomadRunnerManager {
+// NewNomadRunnerManager creates a new runner manager that keeps track of all runners.
+// It uses the apiClient for all requests and runs a background task to keep the runners in sync with Nomad.
+// If you cancel the context the background synchronization will be stopped.
+func NewNomadRunnerManager(apiClient nomad.ExecutorAPI, ctx context.Context) *NomadRunnerManager {
 	m := &NomadRunnerManager{
 		apiClient,
 		NewLocalNomadJobStorage(),
@@ -63,33 +67,34 @@ func NewNomadRunnerManager(apiClient nomad.ExecutorApi, ctx context.Context) *No
 }
 
 type NomadJob struct {
-	environmentId           EnvironmentId
+	environmentID           EnvironmentID
-	jobId                   NomadJobId
+	jobID                   NomadJobID
 	idleRunners             Storage
 	desiredIdleRunnersCount uint
 }
 
-func (j *NomadJob) Id() EnvironmentId {
+func (j *NomadJob) ID() EnvironmentID {
-	return j.environmentId
+	return j.environmentID
 }
 
-func (m *NomadRunnerManager) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) {
+func (m *NomadRunnerManager) RegisterEnvironment(environmentID EnvironmentID, nomadJobID NomadJobID,
+	desiredIdleRunnersCount uint) {
 	m.jobs.Add(&NomadJob{
-		environmentId,
+		environmentID,
-		nomadJobId,
+		nomadJobID,
 		NewLocalRunnerStorage(),
 		desiredIdleRunnersCount,
 	})
-	go m.refreshEnvironment(environmentId)
+	go m.refreshEnvironment(environmentID)
 }
 
-func (m *NomadRunnerManager) EnvironmentExists(id EnvironmentId) (ok bool) {
+func (m *NomadRunnerManager) EnvironmentExists(id EnvironmentID) (ok bool) {
 	_, ok = m.jobs.Get(id)
 	return
 }
 
-func (m *NomadRunnerManager) Claim(environmentId EnvironmentId) (Runner, error) {
+func (m *NomadRunnerManager) Claim(environmentID EnvironmentID) (Runner, error) {
-	job, ok := m.jobs.Get(environmentId)
+	job, ok := m.jobs.Get(environmentID)
 	if !ok {
 		return nil, ErrUnknownExecutionEnvironment
 	}
@@ -101,8 +106,8 @@ func (m *NomadRunnerManager) Claim(environmentId EnvironmentId) (Runner, error)
 	return runner, nil
 }
 
-func (m *NomadRunnerManager) Get(runnerId string) (Runner, error) {
+func (m *NomadRunnerManager) Get(runnerID string) (Runner, error) {
-	runner, ok := m.usedRunners.Get(runnerId)
+	runner, ok := m.usedRunners.Get(runnerID)
 	if !ok {
 		return nil, ErrRunnerNotFound
 	}
@@ -119,42 +124,46 @@ func (m *NomadRunnerManager) Return(r Runner) (err error) {
 }
 
 func (m *NomadRunnerManager) updateRunners(ctx context.Context) {
-	onCreate := func(alloc *nomadApi.Allocation) {
+	retries := 0
-		log.WithField("id", alloc.ID).Debug("Allocation started")
+	for ctx.Err() == nil {
-
+		err := m.apiClient.WatchAllocations(ctx, m.onAllocationAdded, m.onAllocationStopped)
-		intJobID, err := strconv.Atoi(alloc.JobID)
+		retries += 1
-		if err != nil {
+		log.WithError(err).Errorf("Stopped updating the runners! Retry %v", retries)
-			return
+		<-time.After(time.Second)
-		}
-
-		job, ok := m.jobs.Get(EnvironmentId(intJobID))
-		if ok {
-			job.idleRunners.Add(NewRunner(alloc.ID))
-		}
 	}
-	onStop := func(alloc *nomadApi.Allocation) {
+}
-		log.WithField("id", alloc.ID).Debug("Allocation stopped")
 
-		intJobID, err := strconv.Atoi(alloc.JobID)
+func (m *NomadRunnerManager) onAllocationAdded(alloc *nomadApi.Allocation) {
-		if err != nil {
+	log.WithField("id", alloc.ID).Debug("Allocation started")
-			return
-		}
 
-		job, ok := m.jobs.Get(EnvironmentId(intJobID))
+	intJobID, err := strconv.Atoi(alloc.JobID)
-		if ok {
-			job.idleRunners.Delete(alloc.ID)
-			m.usedRunners.Delete(alloc.ID)
-		}
-	}
-
-	err := m.apiClient.WatchAllocations(ctx, onCreate, onStop)
 	if err != nil {
-		log.WithError(err).Error("Failed updating runners")
+		return
+	}
+
+	job, ok := m.jobs.Get(EnvironmentID(intJobID))
+	if ok {
+		job.idleRunners.Add(NewNomadAllocation(alloc.ID, m.apiClient))
+	}
+}
+
+func (m *NomadRunnerManager) onAllocationStopped(alloc *nomadApi.Allocation) {
+	log.WithField("id", alloc.ID).Debug("Allocation stopped")
+
+	intJobID, err := strconv.Atoi(alloc.JobID)
+	if err != nil {
+		return
+	}
+
+	m.usedRunners.Delete(alloc.ID)
+	job, ok := m.jobs.Get(EnvironmentID(intJobID))
+	if ok {
+		job.idleRunners.Delete(alloc.ID)
 	}
 }
 
 // Refresh Big ToDo: Improve this function!! State out that it also rescales the job; Provide context to be terminable...
-func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) {
+func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentID) {
 	job, ok := m.jobs.Get(id)
 	if !ok {
 		// this environment does not exist
@@ -162,7 +171,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) {
 	}
 	var lastJobScaling uint = 0
 	for {
-		runners, err := m.apiClient.LoadRunners(string(job.jobId))
+		runners, err := m.apiClient.LoadRunners(string(job.jobID))
 		if err != nil {
 			log.WithError(err).Printf("Failed fetching runners")
 			break
@@ -173,7 +182,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) {
 
 			job.idleRunners.Add(r)
 		}
-		jobScale, err := m.apiClient.JobScale(string(job.jobId))
+		jobScale, err := m.apiClient.JobScale(string(job.jobID))
 		if err != nil {
 			log.WithError(err).Printf("Failed get allocation count")
 			break
@@ -186,7 +195,7 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) {
 		time.Sleep(50 * time.Millisecond)
 		if requiredRunnerCount != lastJobScaling {
 			log.Printf("Set job scaling %d", requiredRunnerCount)
-			err = m.apiClient.SetJobScale(string(job.jobId), requiredRunnerCount, "Runner Requested")
+			err = m.apiClient.SetJobScale(string(job.jobID), requiredRunnerCount, "Runner Requested")
 			if err != nil {
 				log.WithError(err).Printf("Failed set allocation scaling")
 				continue
@@ -196,19 +205,19 @@ func (m *NomadRunnerManager) refreshEnvironment(id EnvironmentId) {
 	}
 }
 
-func (m *NomadRunnerManager) unusedRunners(environmentId EnvironmentId, fetchedRunnerIds []string) (newRunners []Runner) {
+func (m *NomadRunnerManager) unusedRunners(environmentId EnvironmentID, fetchedRunnerIds []string) (newRunners []Runner) {
 	newRunners = make([]Runner, 0)
 	job, ok := m.jobs.Get(environmentId)
 	if !ok {
 		// the environment does not exist, so it won't have any unused runners
 		return
 	}
-	for _, runnerId := range fetchedRunnerIds {
+	for _, runnerID := range fetchedRunnerIds {
-		_, ok := m.usedRunners.Get(runnerId)
+		_, ok := m.usedRunners.Get(runnerID)
 		if !ok {
-			_, ok = job.idleRunners.Get(runnerId)
+			_, ok = job.idleRunners.Get(runnerID)
 			if !ok {
-				newRunners = append(newRunners, NewNomadAllocation(runnerId, m.apiClient))
+				newRunners = append(newRunners, NewNomadAllocation(runnerID, m.apiClient))
 			}
 		}
 	}

runner/manager_mock.go

@@ -10,11 +10,11 @@ type ManagerMock struct {
 }
 
 // Claim provides a mock function with given fields: id
-func (_m *ManagerMock) Claim(id EnvironmentId) (Runner, error) {
+func (_m *ManagerMock) Claim(id EnvironmentID) (Runner, error) {
 	ret := _m.Called(id)
 
 	var r0 Runner
-	if rf, ok := ret.Get(0).(func(EnvironmentId) Runner); ok {
+	if rf, ok := ret.Get(0).(func(EnvironmentID) Runner); ok {
 		r0 = rf(id)
 	} else {
 		if ret.Get(0) != nil {
@@ -23,7 +23,7 @@ func (_m *ManagerMock) Claim(id EnvironmentId) (Runner, error) {
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(EnvironmentId) error); ok {
+	if rf, ok := ret.Get(1).(func(EnvironmentID) error); ok {
 		r1 = rf(id)
 	} else {
 		r1 = ret.Error(1)
@@ -33,11 +33,11 @@ func (_m *ManagerMock) Claim(id EnvironmentId) (Runner, error) {
 }
 
 // EnvironmentExists provides a mock function with given fields: id
-func (_m *ManagerMock) EnvironmentExists(id EnvironmentId) bool {
+func (_m *ManagerMock) EnvironmentExists(id EnvironmentID) bool {
 	ret := _m.Called(id)
 
 	var r0 bool
-	if rf, ok := ret.Get(0).(func(EnvironmentId) bool); ok {
+	if rf, ok := ret.Get(0).(func(EnvironmentID) bool); ok {
 		r0 = rf(id)
 	} else {
 		r0 = ret.Get(0).(bool)
@@ -46,13 +46,13 @@ func (_m *ManagerMock) EnvironmentExists(id EnvironmentId) bool {
 	return r0
 }
 
-// Get provides a mock function with given fields: runnerId
+// Get provides a mock function with given fields: runnerID
-func (_m *ManagerMock) Get(runnerId string) (Runner, error) {
+func (_m *ManagerMock) Get(runnerID string) (Runner, error) {
-	ret := _m.Called(runnerId)
+	ret := _m.Called(runnerID)
 
 	var r0 Runner
 	if rf, ok := ret.Get(0).(func(string) Runner); ok {
-		r0 = rf(runnerId)
+		r0 = rf(runnerID)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(Runner)
@@ -61,7 +61,7 @@ func (_m *ManagerMock) Get(runnerId string) (Runner, error) {
 
 	var r1 error
 	if rf, ok := ret.Get(1).(func(string) error); ok {
-		r1 = rf(runnerId)
+		r1 = rf(runnerID)
 	} else {
 		r1 = ret.Error(1)
 	}
@@ -69,9 +69,9 @@ func (_m *ManagerMock) Get(runnerId string) (Runner, error) {
 	return r0, r1
 }
 
-// RegisterEnvironment provides a mock function with given fields: environmentId, nomadJobId, desiredIdleRunnersCount
+// RegisterEnvironment provides a mock function with given fields: id, nomadJobID, desiredIdleRunnersCount
-func (_m *ManagerMock) RegisterEnvironment(environmentId EnvironmentId, nomadJobId NomadJobId, desiredIdleRunnersCount uint) {
+func (_m *ManagerMock) RegisterEnvironment(id EnvironmentID, nomadJobID NomadJobID, desiredIdleRunnersCount uint) {
-	_m.Called(environmentId, nomadJobId, desiredIdleRunnersCount)
+	_m.Called(id, nomadJobID, desiredIdleRunnersCount)
 }
 
 // Return provides a mock function with given fields: r

runner/manager_test.go

@@ -32,90 +32,98 @@ type ManagerTestSuite struct {
 
 func (s *ManagerTestSuite) SetupTest() {
 	s.apiMock = &nomad.ExecutorApiMock{}
-	s.nomadRunnerManager = NewNomadRunnerManager(s.apiMock, context.Background())
+	// Instantly closed context to manually start the update process in some cases
-	s.exerciseRunner = NewRunner(tests.DefaultRunnerId)
+	ctx, cancel := context.WithCancel(context.Background())
-	s.mockRunnerQueries([]string{})
+	cancel()
+	s.nomadRunnerManager = NewNomadRunnerManager(s.apiMock, ctx)
+
+	s.exerciseRunner = NewRunner(tests.DefaultRunnerID)
+	mockRunnerQueries(s.apiMock, []string{})
 	s.registerDefaultEnvironment()
 }
 
-func (s *ManagerTestSuite) mockRunnerQueries(returnedRunnerIds []string) {
+func mockRunnerQueries(apiMock *nomad.ExecutorApiMock, returnedRunnerIds []string) {
 	// reset expected calls to allow new mocked return values
-	s.apiMock.ExpectedCalls = []*mock.Call{}
+	apiMock.ExpectedCalls = []*mock.Call{}
-	s.apiMock.On("WatchAllocations", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	call := apiMock.On("WatchAllocations", mock.Anything, mock.Anything, mock.Anything)
-	s.apiMock.On("LoadRunners", tests.DefaultJobId).Return(returnedRunnerIds, nil)
+	call.Run(func(args mock.Arguments) {
-	s.apiMock.On("JobScale", tests.DefaultJobId).Return(len(returnedRunnerIds), nil)
+		<-time.After(10 * time.Minute) // 10 minutes is the default test timeout
-	s.apiMock.On("SetJobScale", tests.DefaultJobId, mock.AnythingOfType("uint"), "Runner Requested").Return(nil)
+		call.ReturnArguments = mock.Arguments{nil}
+	})
+	apiMock.On("LoadRunners", tests.DefaultJobID).Return(returnedRunnerIds, nil)
+	apiMock.On("JobScale", tests.DefaultJobID).Return(uint(len(returnedRunnerIds)), nil)
+	apiMock.On("SetJobScale", tests.DefaultJobID, mock.AnythingOfType("uint"), "Runner Requested").Return(nil)
 }
 
 func (s *ManagerTestSuite) registerDefaultEnvironment() {
-	s.nomadRunnerManager.RegisterEnvironment(defaultEnvironmentId, tests.DefaultJobId, defaultDesiredRunnersCount)
+	s.nomadRunnerManager.RegisterEnvironment(defaultEnvironmentID, tests.DefaultJobID, defaultDesiredRunnersCount)
 }
 
 func (s *ManagerTestSuite) AddIdleRunnerForDefaultEnvironment(r Runner) {
-	job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId)
+	job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID)
 	job.idleRunners.Add(r)
 }
 
 func (s *ManagerTestSuite) waitForRunnerRefresh() {
-	time.Sleep(100 * time.Millisecond)
+	<-time.After(100 * time.Millisecond)
 }
 
 func (s *ManagerTestSuite) TestRegisterEnvironmentAddsNewJob() {
-	s.nomadRunnerManager.RegisterEnvironment(anotherEnvironmentId, tests.DefaultJobId, defaultDesiredRunnersCount)
+	s.nomadRunnerManager.RegisterEnvironment(anotherEnvironmentID, tests.DefaultJobID, defaultDesiredRunnersCount)
-	job, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId)
+	job, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID)
 	s.True(ok)
 	s.NotNil(job)
 }
 
 func (s *ManagerTestSuite) TestClaimReturnsNotFoundErrorIfEnvironmentNotFound() {
-	runner, err := s.nomadRunnerManager.Claim(EnvironmentId(42))
+	runner, err := s.nomadRunnerManager.Claim(EnvironmentID(42))
 	s.Nil(runner)
 	s.Equal(ErrUnknownExecutionEnvironment, err)
 }
 
 func (s *ManagerTestSuite) TestClaimReturnsRunnerIfAvailable() {
 	s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner)
-	receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID)
 	s.NoError(err)
 	s.Equal(s.exerciseRunner, receivedRunner)
 }
 
 func (s *ManagerTestSuite) TestClaimReturnsErrorIfNoRunnerAvailable() {
 	s.waitForRunnerRefresh()
-	runner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	runner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID)
 	s.Nil(runner)
 	s.Equal(ErrNoRunnersAvailable, err)
 }
 
 func (s *ManagerTestSuite) TestClaimReturnsNoRunnerOfDifferentEnvironment() {
 	s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner)
-	receivedRunner, err := s.nomadRunnerManager.Claim(anotherEnvironmentId)
+	receivedRunner, err := s.nomadRunnerManager.Claim(anotherEnvironmentID)
 	s.Nil(receivedRunner)
 	s.Error(err)
 }
 
 func (s *ManagerTestSuite) TestClaimDoesNotReturnTheSameRunnerTwice() {
 	s.AddIdleRunnerForDefaultEnvironment(s.exerciseRunner)
-	s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerId))
+	s.AddIdleRunnerForDefaultEnvironment(NewRunner(tests.AnotherRunnerID))
 
-	firstReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	firstReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID)
-	require.NoError(s.T(), err)
+	s.NoError(err)
-	secondReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	secondReceivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID)
-	require.NoError(s.T(), err)
+	s.NoError(err)
 	s.NotEqual(firstReceivedRunner, secondReceivedRunner)
 }
 
 func (s *ManagerTestSuite) TestClaimThrowsAnErrorIfNoRunnersAvailable() {
-	receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID)
 	s.Nil(receivedRunner)
 	s.Error(err)
 }
 
 func (s *ManagerTestSuite) TestClaimAddsRunnerToUsedRunners() {
-	s.mockRunnerQueries([]string{tests.DefaultRunnerId})
+	mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID})
 	s.waitForRunnerRefresh()
-	receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	receivedRunner, err := s.nomadRunnerManager.Claim(defaultEnvironmentID)
-	require.NoError(s.T(), err)
+	s.Require().NoError(err)
 	savedRunner, ok := s.nomadRunnerManager.usedRunners.Get(receivedRunner.Id())
 	s.True(ok)
 	s.Equal(savedRunner, receivedRunner)
@@ -129,7 +137,7 @@ func (s *ManagerTestSuite) TestGetReturnsRunnerIfRunnerIsUsed() {
 }
 
 func (s *ManagerTestSuite) TestGetReturnsErrorIfRunnerNotFound() {
-	savedRunner, err := s.nomadRunnerManager.Get(tests.DefaultRunnerId)
+	savedRunner, err := s.nomadRunnerManager.Get(tests.DefaultRunnerID)
 	s.Nil(savedRunner)
 	s.Error(err)
 }
@@ -157,94 +165,107 @@ func (s *ManagerTestSuite) TestReturnReturnsErrorWhenApiCallFailed() {
 }
 
 func (s *ManagerTestSuite) TestRefreshFetchesRunners() {
-	s.mockRunnerQueries([]string{tests.DefaultRunnerId})
+	mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID})
 	s.waitForRunnerRefresh()
-	s.apiMock.AssertCalled(s.T(), "LoadRunners", tests.DefaultJobId)
+	s.apiMock.AssertCalled(s.T(), "LoadRunners", tests.DefaultJobID)
 }
 
 func (s *ManagerTestSuite) TestNewRunnersFoundInRefreshAreAddedToIdleRunners() {
-	s.mockRunnerQueries([]string{tests.DefaultRunnerId})
+	mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID})
 	s.waitForRunnerRefresh()
-	job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId)
+	job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID)
-	_, ok := job.idleRunners.Get(tests.DefaultRunnerId)
+	_, ok := job.idleRunners.Get(tests.DefaultRunnerID)
 	s.True(ok)
 }
 
 func (s *ManagerTestSuite) TestRefreshScalesJob() {
-	s.mockRunnerQueries([]string{tests.DefaultRunnerId})
+	mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID})
 	s.waitForRunnerRefresh()
 	// use one runner to necessitate rescaling
-	_, _ = s.nomadRunnerManager.Claim(defaultEnvironmentId)
+	_, _ = s.nomadRunnerManager.Claim(defaultEnvironmentID)
 	s.waitForRunnerRefresh()
-	s.apiMock.AssertCalled(s.T(), "SetJobScale", tests.DefaultJobId, defaultDesiredRunnersCount, "Runner Requested")
+	s.apiMock.AssertCalled(s.T(), "SetJobScale", tests.DefaultJobID, defaultDesiredRunnersCount, "Runner Requested")
 }
 
 func (s *ManagerTestSuite) TestRefreshAddsRunnerToPool() {
-	s.mockRunnerQueries([]string{tests.DefaultRunnerId})
+	mockRunnerQueries(s.apiMock, []string{tests.DefaultRunnerID})
 	s.waitForRunnerRefresh()
-	job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId)
+	job, _ := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID)
-	poolRunner, ok := job.idleRunners.Get(tests.DefaultRunnerId)
+	poolRunner, ok := job.idleRunners.Get(tests.DefaultRunnerID)
 	s.True(ok)
-	s.Equal(tests.DefaultRunnerId, poolRunner.Id())
+	s.Equal(tests.DefaultRunnerID, poolRunner.Id())
 }
 
 func (s *ManagerTestSuite) TestUpdateRunnersLogsErrorFromWatchAllocation() {
 	var hook *test.Hook
 	logger, hook := test.NewNullLogger()
 	log = logger.WithField("pkg", "runner")
-	s.modifyMockedCall("WatchAllocations", func(call *mock.Call) {
+	modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) {
-		call.Return(tests.DefaultError)
-	})
-
-	s.nomadRunnerManager.updateRunners(context.Background())
-
-	require.Equal(s.T(), 1, len(hook.Entries))
-	s.Equal(logrus.ErrorLevel, hook.LastEntry().Level)
-	s.Equal(hook.LastEntry().Data[logrus.ErrorKey], tests.DefaultError)
-}
-
-func (s *ManagerTestSuite) TestUpdateRunnersAddsIdleRunner() {
-	allocation := &nomadApi.Allocation{ID: tests.AllocationID}
-	defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId)
-	require.True(s.T(), ok)
-	allocation.JobID = string(defaultJob.jobId)
-
-	_, ok = defaultJob.idleRunners.Get(allocation.ID)
-	require.False(s.T(), ok)
-
-	s.modifyMockedCall("WatchAllocations", func(call *mock.Call) {
 		call.Run(func(args mock.Arguments) {
-			onCreate, ok := args.Get(1).(nomad.AllocationProcessor)
+			call.ReturnArguments = mock.Arguments{tests.ErrDefault}
-			require.True(s.T(), ok)
-			onCreate(allocation)
 		})
 	})
 
-	s.nomadRunnerManager.updateRunners(context.Background())
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go s.nomadRunnerManager.updateRunners(ctx)
+	<-time.After(10 * time.Millisecond)
+
+	s.Require().Equal(1, len(hook.Entries))
+	s.Equal(logrus.ErrorLevel, hook.LastEntry().Level)
+	s.Equal(hook.LastEntry().Data[logrus.ErrorKey], tests.ErrDefault)
+}
+
+func (s *ManagerTestSuite) TestUpdateRunnersAddsIdleRunner() {
+	allocation := &nomadApi.Allocation{ID: tests.DefaultRunnerID}
+	defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID)
+	s.Require().True(ok)
+	allocation.JobID = string(defaultJob.jobID)
+
+	_, ok = defaultJob.idleRunners.Get(allocation.ID)
+	s.Require().False(ok)
+
+	modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) {
+		call.Run(func(args mock.Arguments) {
+			onCreate, ok := args.Get(1).(nomad.AllocationProcessor)
+			s.Require().True(ok)
+			onCreate(allocation)
+			call.ReturnArguments = mock.Arguments{nil}
+		})
+	})
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go s.nomadRunnerManager.updateRunners(ctx)
+	<-time.After(10 * time.Millisecond)
 
 	_, ok = defaultJob.idleRunners.Get(allocation.ID)
 	s.True(ok)
 }
 
 func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() {
-	allocation := &nomadApi.Allocation{ID: tests.AllocationID}
+	allocation := &nomadApi.Allocation{ID: tests.DefaultRunnerID}
-	defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentId)
+	defaultJob, ok := s.nomadRunnerManager.jobs.Get(defaultEnvironmentID)
-	require.True(s.T(), ok)
+	s.Require().True(ok)
-	allocation.JobID = string(defaultJob.jobId)
+	allocation.JobID = string(defaultJob.jobID)
 
 	testRunner := NewRunner(allocation.ID)
 	defaultJob.idleRunners.Add(testRunner)
 	s.nomadRunnerManager.usedRunners.Add(testRunner)
 
-	s.modifyMockedCall("WatchAllocations", func(call *mock.Call) {
+	modifyMockedCall(s.apiMock, "WatchAllocations", func(call *mock.Call) {
 		call.Run(func(args mock.Arguments) {
 			onDelete, ok := args.Get(2).(nomad.AllocationProcessor)
-			require.True(s.T(), ok)
+			s.Require().True(ok)
 			onDelete(allocation)
+			call.ReturnArguments = mock.Arguments{nil}
 		})
 	})
 
-	s.nomadRunnerManager.updateRunners(context.Background())
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go s.nomadRunnerManager.updateRunners(ctx)
+	<-time.After(10 * time.Millisecond)
 
 	_, ok = defaultJob.idleRunners.Get(allocation.ID)
 	s.False(ok)
@@ -252,18 +273,16 @@ func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() {
 	s.False(ok)
 }
 
-func (s *ManagerTestSuite) modifyMockedCall(method string, modifier func(call *mock.Call)) {
+func modifyMockedCall(apiMock *nomad.ExecutorApiMock, method string, modifier func(call *mock.Call)) {
-	for _, c := range s.apiMock.ExpectedCalls {
+	for _, c := range apiMock.ExpectedCalls {
 		if c.Method == method {
 			modifier(c)
 		}
 	}
-	s.True(ok)
-	s.Equal(tests.DefaultRunnerId, poolRunner.Id())
 }
 
 func (s *ManagerTestSuite) TestWhenEnvironmentDoesNotExistEnvironmentExistsReturnsFalse() {
-	id := anotherEnvironmentId
+	id := anotherEnvironmentID
 	_, ok := s.nomadRunnerManager.jobs.Get(id)
 	require.False(s.T(), ok)
 
@@ -271,8 +290,8 @@ func (s *ManagerTestSuite) TestWhenEnvironmentDoesNotExistEnvironmentExistsRetur
 }
 
 func (s *ManagerTestSuite) TestWhenEnvironmentExistsEnvironmentExistsReturnsTrue() {
-	id := anotherEnvironmentId
+	id := anotherEnvironmentID
-	s.nomadRunnerManager.jobs.Add(&NomadJob{environmentId: id})
+	s.nomadRunnerManager.jobs.Add(&NomadJob{environmentID: id})
 
 	exists := s.nomadRunnerManager.EnvironmentExists(id)
 	s.True(exists)

runner/nomad_job_storage.go

@@ -12,10 +12,11 @@ type NomadJobStorage interface {
 
 	// Get returns a job from the storage.
 	// Iff the job does not exist in the store, ok will be false.
-	Get(id EnvironmentId) (job *NomadJob, ok bool)
+	Get(id EnvironmentID) (job *NomadJob, ok bool)
 
-	// Delete deletes the job with the passed id from the storage. It does nothing if no job with the id is present in the storage.
+	// Delete deletes the job with the passed id from the storage. It does nothing if no job with the id is present in
-	Delete(id EnvironmentId)
+	// the storage.
+	Delete(id EnvironmentID)
 
 	// Length returns the number of currently stored jobs in the storage.
 	Length() int
@@ -24,31 +25,31 @@ type NomadJobStorage interface {
 // localNomadJobStorage stores NomadJob objects in the local application memory.
 type localNomadJobStorage struct {
 	sync.RWMutex
-	jobs map[EnvironmentId]*NomadJob
+	jobs map[EnvironmentID]*NomadJob
 }
 
 // NewLocalNomadJobStorage responds with an empty localNomadJobStorage.
 // This implementation stores the data thread-safe in the local application memory.
 func NewLocalNomadJobStorage() *localNomadJobStorage {
 	return &localNomadJobStorage{
-		jobs: make(map[EnvironmentId]*NomadJob),
+		jobs: make(map[EnvironmentID]*NomadJob),
 	}
 }
 
 func (s *localNomadJobStorage) Add(job *NomadJob) {
 	s.Lock()
 	defer s.Unlock()
-	s.jobs[job.Id()] = job
+	s.jobs[job.ID()] = job
 }
 
-func (s *localNomadJobStorage) Get(id EnvironmentId) (job *NomadJob, ok bool) {
+func (s *localNomadJobStorage) Get(id EnvironmentID) (job *NomadJob, ok bool) {
 	s.RLock()
 	defer s.RUnlock()
 	job, ok = s.jobs[id]
 	return
 }
 
-func (s *localNomadJobStorage) Delete(id EnvironmentId) {
+func (s *localNomadJobStorage) Delete(id EnvironmentID) {
 	s.Lock()
 	defer s.Unlock()
 	delete(s.jobs, id)

runner/nomad_job_storage_test.go

@@ -16,62 +16,62 @@ type JobStoreTestSuite struct {
 	job        *NomadJob
 }
 
-func (suite *JobStoreTestSuite) SetupTest() {
+func (s *JobStoreTestSuite) SetupTest() {
-	suite.jobStorage = NewLocalNomadJobStorage()
+	s.jobStorage = NewLocalNomadJobStorage()
-	suite.job = &NomadJob{environmentId: defaultEnvironmentId, jobId: tests.DefaultJobId}
+	s.job = &NomadJob{environmentID: defaultEnvironmentID, jobID: tests.DefaultJobID}
 }
 
-func (suite *JobStoreTestSuite) TestAddedJobCanBeRetrieved() {
+func (s *JobStoreTestSuite) TestAddedJobCanBeRetrieved() {
-	suite.jobStorage.Add(suite.job)
+	s.jobStorage.Add(s.job)
-	retrievedJob, ok := suite.jobStorage.Get(suite.job.Id())
+	retrievedJob, ok := s.jobStorage.Get(s.job.ID())
-	suite.True(ok, "A saved runner should be retrievable")
+	s.True(ok, "A saved runner should be retrievable")
-	suite.Equal(suite.job, retrievedJob)
+	s.Equal(s.job, retrievedJob)
 }
 
-func (suite *JobStoreTestSuite) TestJobWithSameIdOverwritesOldOne() {
+func (s *JobStoreTestSuite) TestJobWithSameIdOverwritesOldOne() {
-	otherJobWithSameId := &NomadJob{environmentId: defaultEnvironmentId}
+	otherJobWithSameID := &NomadJob{environmentID: defaultEnvironmentID}
 	// assure runner is actually different
-	otherJobWithSameId.jobId = tests.AnotherJobId
+	otherJobWithSameID.jobID = tests.AnotherJobID
-	suite.NotEqual(suite.job, otherJobWithSameId)
+	s.NotEqual(s.job, otherJobWithSameID)
 
-	suite.jobStorage.Add(suite.job)
+	s.jobStorage.Add(s.job)
-	suite.jobStorage.Add(otherJobWithSameId)
+	s.jobStorage.Add(otherJobWithSameID)
-	retrievedJob, _ := suite.jobStorage.Get(suite.job.Id())
+	retrievedJob, _ := s.jobStorage.Get(s.job.ID())
-	suite.NotEqual(suite.job, retrievedJob)
+	s.NotEqual(s.job, retrievedJob)
-	suite.Equal(otherJobWithSameId, retrievedJob)
+	s.Equal(otherJobWithSameID, retrievedJob)
 }
 
-func (suite *JobStoreTestSuite) TestDeletedJobIsNotAccessible() {
+func (s *JobStoreTestSuite) TestDeletedJobIsNotAccessible() {
-	suite.jobStorage.Add(suite.job)
+	s.jobStorage.Add(s.job)
-	suite.jobStorage.Delete(suite.job.Id())
+	s.jobStorage.Delete(s.job.ID())
-	retrievedRunner, ok := suite.jobStorage.Get(suite.job.Id())
+	retrievedRunner, ok := s.jobStorage.Get(s.job.ID())
-	suite.Nil(retrievedRunner)
+	s.Nil(retrievedRunner)
-	suite.False(ok, "A deleted runner should not be accessible")
+	s.False(ok, "A deleted runner should not be accessible")
 }
 
-func (suite *JobStoreTestSuite) TestLenOfEmptyPoolIsZero() {
+func (s *JobStoreTestSuite) TestLenOfEmptyPoolIsZero() {
-	suite.Equal(0, suite.jobStorage.Length())
+	s.Equal(0, s.jobStorage.Length())
 }
 
-func (suite *JobStoreTestSuite) TestLenChangesOnStoreContentChange() {
+func (s *JobStoreTestSuite) TestLenChangesOnStoreContentChange() {
-	suite.Run("len increases when job is added", func() {
+	s.Run("len increases when job is added", func() {
-		suite.jobStorage.Add(suite.job)
+		s.jobStorage.Add(s.job)
-		suite.Equal(1, suite.jobStorage.Length())
+		s.Equal(1, s.jobStorage.Length())
 	})
 
-	suite.Run("len does not increase when job with same id is added", func() {
+	s.Run("len does not increase when job with same id is added", func() {
-		suite.jobStorage.Add(suite.job)
+		s.jobStorage.Add(s.job)
-		suite.Equal(1, suite.jobStorage.Length())
+		s.Equal(1, s.jobStorage.Length())
 	})
 
-	suite.Run("len increases again when different job is added", func() {
+	s.Run("len increases again when different job is added", func() {
-		anotherJob := &NomadJob{environmentId: anotherEnvironmentId}
+		anotherJob := &NomadJob{environmentID: anotherEnvironmentID}
-		suite.jobStorage.Add(anotherJob)
+		s.jobStorage.Add(anotherJob)
-		suite.Equal(2, suite.jobStorage.Length())
+		s.Equal(2, s.jobStorage.Length())
 	})
 
-	suite.Run("len decreases when job is deleted", func() {
+	s.Run("len decreases when job is deleted", func() {
-		suite.jobStorage.Delete(suite.job.Id())
+		s.jobStorage.Delete(s.job.ID())
-		suite.Equal(1, suite.jobStorage.Length())
+		s.Equal(1, s.jobStorage.Length())
 	})
 }

runner/runner.go

@@ -54,16 +54,11 @@ type Runner interface {
 type NomadAllocation struct {
 	ExecutionStorage
 	id  string
-	api nomad.ExecutorApi
+	api nomad.ExecutorAPI
-}
-
-// NewRunner creates a new runner with the provided id.
-func NewRunner(id string) Runner {
-	return NewNomadAllocation(id, nil)
 }
 
 // NewNomadAllocation creates a new Nomad allocation with the provided id.
-func NewNomadAllocation(id string, apiClient nomad.ExecutorApi) *NomadAllocation {
+func NewNomadAllocation(id string, apiClient nomad.ExecutorAPI) *NomadAllocation {
 	return &NomadAllocation{
 		id:               id,
 		api:              apiClient,
@@ -189,9 +184,9 @@ func tarHeader(file dto.File) *tar.Header {
 // This exports private attributes like the id too.
 func (r *NomadAllocation) MarshalJSON() ([]byte, error) {
 	return json.Marshal(struct {
-		Id string `json:"runnerId"`
+		ID string `json:"runnerId"`
 	}{
-		Id: r.Id(),
+		ID: r.Id(),
 	})
 }
 

runner/runner_test.go

@@ -75,18 +75,18 @@ func TestFromContextReturnsIsNotOkWhenContextHasNoRunner(t *testing.T) {
 func TestExecuteCallsAPI(t *testing.T) {
 	apiMock := &nomad.ExecutorApiMock{}
 	apiMock.On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, true, mock.Anything, mock.Anything, mock.Anything).Return(0, nil)
-	runner := NewNomadAllocation(tests.DefaultRunnerId, apiMock)
+	runner := NewNomadAllocation(tests.DefaultRunnerID, apiMock)
 
 	request := &dto.ExecutionRequest{Command: "echo 'Hello World!'"}
 	runner.ExecuteInteractively(request, nil, nil, nil)
 
 	<-time.After(50 * time.Millisecond)
-	apiMock.AssertCalled(t, "ExecuteCommand", tests.DefaultRunnerId, mock.Anything, request.FullCommand(), true, mock.Anything, mock.Anything, mock.Anything)
+	apiMock.AssertCalled(t, "ExecuteCommand", tests.DefaultRunnerID, mock.Anything, request.FullCommand(), true, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestExecuteReturnsAfterTimeout(t *testing.T) {
 	apiMock := newApiMockWithTimeLimitHandling()
-	runner := NewNomadAllocation(tests.DefaultRunnerId, apiMock)
+	runner := NewNomadAllocation(tests.DefaultRunnerID, apiMock)
 
 	timeLimit := 1
 	execution := &dto.ExecutionRequest{TimeLimit: timeLimit}
@@ -133,8 +133,8 @@ type UpdateFileSystemTestSuite struct {
 
 func (s *UpdateFileSystemTestSuite) SetupTest() {
 	s.apiMock = &nomad.ExecutorApiMock{}
-	s.runner = NewNomadAllocation(tests.DefaultRunnerId, s.apiMock)
+	s.runner = NewNomadAllocation(tests.DefaultRunnerID, s.apiMock)
-	s.mockedExecuteCommandCall = s.apiMock.On("ExecuteCommand", tests.DefaultRunnerId, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything).
+	s.mockedExecuteCommandCall = s.apiMock.On("ExecuteCommand", tests.DefaultRunnerID, mock.Anything, mock.Anything, false, mock.Anything, mock.Anything, mock.Anything).
 		Run(func(args mock.Arguments) {
 			s.command = args.Get(2).([]string)
 			s.stdin = args.Get(4).(*bytes.Buffer)
@@ -160,7 +160,7 @@ func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerReturnsErrorIfE
 }
 
 func (s *UpdateFileSystemTestSuite) TestUpdateFileSystemForRunnerReturnsErrorIfApiCallDid() {
-	s.mockedExecuteCommandCall.Return(0, tests.DefaultError)
+	s.mockedExecuteCommandCall.Return(0, tests.ErrDefault)
 	copyRequest := &dto.UpdateFileSystemRequest{}
 	err := s.runner.UpdateFileSystem(copyRequest)
 	s.ErrorIs(err, nomad.ErrorExecutorCommunicationFailed)
@@ -251,3 +251,8 @@ func (s *UpdateFileSystemTestSuite) readFilesFromTarArchive(tarArchive io.Reader
 	}
 	return files
 }
+
+// NewRunner creates a new runner with the provided id.
+func NewRunner(id string) Runner {
+	return NewNomadAllocation(id, nil)
+}

runner/storage_test.go

@@ -19,8 +19,8 @@ type RunnerPoolTestSuite struct {
 
 func (suite *RunnerPoolTestSuite) SetupTest() {
 	suite.runnerStorage = NewLocalRunnerStorage()
-	suite.runner = NewRunner(tests.DefaultRunnerId)
+	suite.runner = NewRunner(tests.DefaultRunnerID)
-	suite.runner.Add(tests.DefaultExecutionId, &dto.ExecutionRequest{Command: "true"})
+	suite.runner.Add(tests.DefaultExecutionID, &dto.ExecutionRequest{Command: "true"})
 }
 
 func (suite *RunnerPoolTestSuite) TestAddedRunnerCanBeRetrieved() {
@@ -86,7 +86,7 @@ func (suite *RunnerPoolTestSuite) TestLenChangesOnStoreContentChange() {
 	})
 
 	suite.Run("len increases again when different runner is added", func() {
-		anotherRunner := NewRunner(tests.AnotherRunnerId)
+		anotherRunner := NewRunner(tests.AnotherRunnerID)
 		suite.runnerStorage.Add(anotherRunner)
 		suite.Equal(2, suite.runnerStorage.Length())
 	})

tests/constants.go

@@ -3,23 +3,23 @@ package tests
 import "errors"
 
 const (
-	NonExistingId                 = "n0n-3x1st1ng-1d"
+	NonExistingID                 = "n0n-3x1st1ng-1d"
 	DefaultFileName               = "test.txt"
 	DefaultFileContent            = "Hello, Codemoon!"
 	DefaultDirectoryName          = "test/"
 	FileNameWithAbsolutePath      = "/test.txt"
-	DefaultEnvironmentIdAsInteger = 0
+	DefaultEnvironmentIDAsInteger = 0
-	DefaultEnvironmentIdAsString  = "0"
+	DefaultEnvironmentIDAsString  = "0"
-	AnotherEnvironmentIdAsInteger = 42
+	AnotherEnvironmentIDAsInteger = 42
-	AnotherEnvironmentIdAsString  = "42"
+	AnotherEnvironmentIDAsString  = "42"
-	DefaultJobId                  = "s0m3-j0b-1d"
+	DefaultJobID                  = DefaultEnvironmentIDAsString
-	AnotherJobId                  = "4n0th3r-j0b-1d"
+	AnotherJobID                  = AnotherEnvironmentIDAsString
-	DefaultRunnerId               = "s0m3-r4nd0m-1d"
+	DefaultRunnerID               = DefaultJobID
-	AnotherRunnerId               = "4n0th3r-runn3r-1d"
+	AnotherRunnerID               = AnotherJobID
-	DefaultExecutionId            = "s0m3-3x3cu710n-1d"
+	DefaultExecutionID            = "s0m3-3x3cu710n-1d"
-	DefaultMockId                 = "m0ck-1d"
+	DefaultMockID                 = "m0ck-1d"
 )
 
 var (
-	DefaultError = errors.New("an error occurred")
+	ErrDefault = errors.New("an error occurred")
 )

tests/e2e/environments_test.go

@@ -18,7 +18,7 @@ const (
 )
 
 func TestCreateOrUpdateEnvironment(t *testing.T) {
-	path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIdAsString)
+	path := helpers.BuildURL(api.BasePath, api.EnvironmentsPath, tests.AnotherEnvironmentIDAsString)
 
 	t.Run("returns bad request with empty body", func(t *testing.T) {
 		resp, err := helpers.HttpPut(path, strings.NewReader(""))
@@ -68,7 +68,7 @@ func TestCreateOrUpdateEnvironment(t *testing.T) {
 	})
 
 	_, _, err := nomadClient.Jobs().DeregisterOpts(
-		tests.AnotherEnvironmentIdAsString, &nomadApi.DeregisterOptions{Purge: true}, nil)
+		tests.AnotherEnvironmentIDAsString, &nomadApi.DeregisterOptions{Purge: true}, nil)
 	if err != nil {
 		t.Fatalf("Error when removing test job %v", err)
 	}
@@ -87,7 +87,7 @@ func assertPutReturnsStatusAndZeroContent(t *testing.T, path string,
 
 func validateJob(t *testing.T, expected dto.ExecutionEnvironmentRequest) {
 	t.Helper()
-	job := findNomadJob(t, tests.AnotherEnvironmentIdAsString)
+	job := findNomadJob(t, tests.AnotherEnvironmentIDAsString)
 
 	assertEqualValueStringPointer(t, nomadNamespace, job.Namespace)
 	assertEqualValueStringPointer(t, "batch", job.Type)

tests/e2e/runners_test.go

@@ -66,7 +66,7 @@ func (s *E2ETestSuite) TestDeleteRunnerRoute() {
 	})
 
 	s.Run("Deleting non-existing runner returns NotFound", func() {
-		resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingId), nil)
+		resp, err := helpers.HttpDelete(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingID), nil)
 		s.NoError(err)
 		s.Equal(http.StatusNotFound, resp.StatusCode)
 	})
@@ -178,7 +178,7 @@ func (s *E2ETestSuite) TestCopyFilesRoute() {
 	})
 
 	s.Run("Copying to non-existing runner returns NotFound", func() {
-		resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingId, api.UpdateFileSystemPath), "application/json", bytes.NewReader(copyFilesRequestByteString))
+		resp, err := helpers.HttpPatch(helpers.BuildURL(api.BasePath, api.RunnersPath, tests.NonExistingID, api.UpdateFileSystemPath), "application/json", bytes.NewReader(copyFilesRequestByteString))
 		s.NoError(err)
 		s.Equal(http.StatusNotFound, resp.StatusCode)
 	})