Add tests for LoadAvailableRunners method

api/runners_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/stretchr/testify/suite"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/environment"
-	"gitlab.hpi.de/codeocean/codemoon/poseidon/mocks"
+	"gitlab.hpi.de/codeocean/codemoon/poseidon/nomad"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/runner"
 	"net/http"
 	"net/http/httptest"
@@ -139,7 +139,7 @@ func TestDeleteRunnerRouteTestSuite(t *testing.T) {
 type DeleteRunnerRouteTestSuite struct {
 	suite.Suite
 	runnerPool environment.RunnerPool
-	apiClient  *mocks.ExecutorApi
+	apiClient  *nomad.ExecutorApiMock
 	router     *mux.Router
 	testRunner runner.Runner
 	path       string
@@ -147,7 +147,7 @@ type DeleteRunnerRouteTestSuite struct {
 
 func (suite *DeleteRunnerRouteTestSuite) SetupTest() {
 	suite.runnerPool = environment.NewLocalRunnerPool()
-	suite.apiClient = &mocks.ExecutorApi{}
+	suite.apiClient = &nomad.ExecutorApiMock{}
 	suite.router = NewRouter(suite.apiClient, suite.runnerPool)
 
 	suite.testRunner = runner.NewExerciseRunner("testRunner")

environment/execution_environment_test.go

@@ -4,7 +4,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
-	"gitlab.hpi.de/codeocean/codemoon/poseidon/mocks"
+	"gitlab.hpi.de/codeocean/codemoon/poseidon/nomad"
 	"gitlab.hpi.de/codeocean/codemoon/poseidon/runner"
 	"testing"
 	"time"
@@ -94,8 +94,8 @@ func TestRefreshAddsRunnerToPool(t *testing.T) {
 	assert.Equal(t, availableRunner, poolRunner)
 }
 
-func newRefreshMock(returnedRunnerIds []string, allRunners RunnerPool) (apiClient *mocks.ExecutorApi, environment *NomadExecutionEnvironment) {
+func newRefreshMock(returnedRunnerIds []string, allRunners RunnerPool) (apiClient *nomad.ExecutorApiMock, environment *NomadExecutionEnvironment) {
-	apiClient = &mocks.ExecutorApi{}
+	apiClient = &nomad.ExecutorApiMock{}
 	apiClient.On("LoadAvailableRunners", jobId).Return(returnedRunnerIds, nil)
 	apiClient.On("GetJobScale", jobId).Return(len(returnedRunnerIds), nil)
 	apiClient.On("SetJobScaling", jobId, mock.AnythingOfType("int"), "Runner Requested").Return(nil)

nomad/ExecutorApiMock.go

@@ -0,0 +1,147 @@
+// Code generated by mockery v0.0.0-dev. DO NOT EDIT.
+
+package nomad
+
+import (
+	api "github.com/hashicorp/nomad/api"
+	mock "github.com/stretchr/testify/mock"
+
+	url "net/url"
+)
+
+// ExecutorApiMock is an autogenerated mock type for the ExecutorApi type
+type ExecutorApiMock struct {
+	mock.Mock
+}
+
+// DeleteRunner provides a mock function with given fields: runnerId
+func (_m *ExecutorApiMock) DeleteRunner(runnerId string) error {
+	ret := _m.Called(runnerId)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string) error); ok {
+		r0 = rf(runnerId)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// GetJobScale provides a mock function with given fields: jobId
+func (_m *ExecutorApiMock) GetJobScale(jobId string) (int, error) {
+	ret := _m.Called(jobId)
+
+	var r0 int
+	if rf, ok := ret.Get(0).(func(string) int); ok {
+		r0 = rf(jobId)
+	} else {
+		r0 = ret.Get(0).(int)
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(jobId)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// LoadAvailableRunners provides a mock function with given fields: jobId
+func (_m *ExecutorApiMock) LoadAvailableRunners(jobId string) ([]string, error) {
+	ret := _m.Called(jobId)
+
+	var r0 []string
+	if rf, ok := ret.Get(0).(func(string) []string); ok {
+		r0 = rf(jobId)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]string)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(jobId)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// LoadJobList provides a mock function with given fields:
+func (_m *ExecutorApiMock) LoadJobList() ([]*api.JobListStub, error) {
+	ret := _m.Called()
+
+	var r0 []*api.JobListStub
+	if rf, ok := ret.Get(0).(func() []*api.JobListStub); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*api.JobListStub)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func() error); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// SetJobScaling provides a mock function with given fields: jobId, count, reason
+func (_m *ExecutorApiMock) SetJobScaling(jobId string, count int, reason string) error {
+	ret := _m.Called(jobId, count, reason)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, int, string) error); ok {
+		r0 = rf(jobId, count, reason)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// init provides a mock function with given fields: nomadURL
+func (_m *ExecutorApiMock) init(nomadURL *url.URL) error {
+	ret := _m.Called(nomadURL)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(*url.URL) error); ok {
+		r0 = rf(nomadURL)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// loadRunners provides a mock function with given fields: jobId
+func (_m *ExecutorApiMock) loadRunners(jobId string) ([]*api.AllocationListStub, error) {
+	ret := _m.Called(jobId)
+
+	var r0 []*api.AllocationListStub
+	if rf, ok := ret.Get(0).(func(string) []*api.AllocationListStub); ok {
+		r0 = rf(jobId)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*api.AllocationListStub)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(jobId)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}

nomad/nomad.go

@@ -7,10 +7,23 @@ import (
 
 // ExecutorApi provides access to an container orchestration solution
 type ExecutorApi interface {
+	nomadApiQuerier
+
+	// LoadAvailableRunners loads all allocations of the specified job which are running and not about to get stopped.
+	LoadAvailableRunners(jobId string) (runnerIds []string, err error)
+}
+
+// nomadApiQuerier provides access to the Nomad functionality
+type nomadApiQuerier interface {
+	// init prepares an apiClient to be able to communicate to a provided Nomad API
+	init(nomadURL *url.URL) (err error)
+
 	// LoadJobList loads the list of jobs from the Nomad api.
 	LoadJobList() (list []*nomadApi.JobListStub, err error)
+
 	// GetJobScale returns the scale of the passed job.
 	GetJobScale(jobId string) (jobScale int, err error)
+
 	// SetJobScaling sets the scaling count of the passed job to Nomad.
 	SetJobScaling(jobId string, count int, reason string) (err error)
 
@@ -18,53 +31,29 @@ type ExecutorApi interface {
 	DeleteRunner(runnerId string) (err error)
 
 	// LoadAvailableRunners loads all allocations of the specified job which are running and not about to get stopped.
-	LoadAvailableRunners(jobId string) (runnerIds []string, err error)
+	loadRunners(jobId string) (allocationListStub []*nomadApi.AllocationListStub, err error)
 }
 
-// ApiClient provides access to the Nomad functionality
 type ApiClient struct {
+	nomadApiQuerier
+}
+
+type directNomadApiClient struct {
 	client *nomadApi.Client
 }
 
 // New creates a new api client.
 // One client is usually sufficient for the complete runtime of the API.
 func New(nomadURL *url.URL) (ExecutorApi, error) {
-	client := &ApiClient{}
+	client := &ApiClient{
+		nomadApiQuerier: &directNomadApiClient{},
+	}
 	err := client.init(nomadURL)
 	return client, err
 }
 
-// init prepares an apiClient to be able to communicate to a provided Nomad API.
-func (apiClient *ApiClient) init(nomadURL *url.URL) (err error) {
-	apiClient.client, err = nomadApi.NewClient(&nomadApi.Config{
-		Address:   nomadURL.String(),
-		TLSConfig: &nomadApi.TLSConfig{},
-	})
-	return err
-}
-
-func (apiClient *ApiClient) LoadJobList() (list []*nomadApi.JobListStub, err error) {
-	list, _, err = apiClient.client.Jobs().List(nil)
-	return
-}
-
-func (apiClient *ApiClient) GetJobScale(jobId string) (jobScale int, err error) {
-	status, _, err := apiClient.client.Jobs().ScaleStatus(jobId, nil)
-	if err != nil {
-		return
-	}
-	// ToDo: Consider counting also the placed and desired allocations
-	jobScale = status.TaskGroups[jobId].Running
-	return
-}
-
-func (apiClient *ApiClient) SetJobScaling(jobId string, count int, reason string) (err error) {
-	_, _, err = apiClient.client.Jobs().Scale(jobId, jobId, &count, reason, false, nil, nil)
-	return
-}
-
 func (apiClient *ApiClient) LoadAvailableRunners(jobId string) (runnerIds []string, err error) {
-	list, _, err := apiClient.client.Jobs().Allocations(jobId, true, nil)
+	list, err := apiClient.loadRunners(jobId)
 	if err != nil {
 		return nil, err
 	}
@@ -77,11 +66,44 @@ func (apiClient *ApiClient) LoadAvailableRunners(jobId string) (runnerIds []stri
 	return
 }
 
-func (apiClient *ApiClient) DeleteRunner(runnerId string) (err error) {
+func (rawApiClient *directNomadApiClient) init(nomadURL *url.URL) (err error) {
-	allocation, _, err := apiClient.client.Allocations().Info(runnerId, nil)
+	rawApiClient.client, err = nomadApi.NewClient(&nomadApi.Config{
+		Address:   nomadURL.String(),
+		TLSConfig: &nomadApi.TLSConfig{},
+	})
+	return err
+}
+
+func (rawApiClient *directNomadApiClient) LoadJobList() (list []*nomadApi.JobListStub, err error) {
+	list, _, err = rawApiClient.client.Jobs().List(nil)
+	return
+}
+
+func (rawApiClient *directNomadApiClient) GetJobScale(jobId string) (jobScale int, err error) {
+	status, _, err := rawApiClient.client.Jobs().ScaleStatus(jobId, nil)
 	if err != nil {
 		return
 	}
-	_, err = apiClient.client.Allocations().Stop(allocation, nil)
+	// ToDo: Consider counting also the placed and desired allocations
+	jobScale = status.TaskGroups[jobId].Running
+	return
+}
+
+func (rawApiClient *directNomadApiClient) SetJobScaling(jobId string, count int, reason string) (err error) {
+	_, _, err = rawApiClient.client.Jobs().Scale(jobId, jobId, &count, reason, false, nil, nil)
+	return
+}
+
+func (rawApiClient *directNomadApiClient) loadRunners(jobId string) (allocationListStub []*nomadApi.AllocationListStub, err error) {
+	allocationListStub, _, err = rawApiClient.client.Jobs().Allocations(jobId, true, nil)
+	return
+}
+
+func (rawApiClient *directNomadApiClient) DeleteRunner(runnerId string) (err error) {
+	allocation, _, err := rawApiClient.client.Allocations().Info(runnerId, nil)
+	if err != nil {
+		return
+	}
+	_, err = rawApiClient.client.Allocations().Stop(allocation, nil)
 	return err
 }

nomad/nomadApiQuerierMock.go

@@ -1,19 +1,21 @@
 // Code generated by mockery v0.0.0-dev. DO NOT EDIT.
 
-package mocks
+package nomad
 
 import (
 	api "github.com/hashicorp/nomad/api"
 	mock "github.com/stretchr/testify/mock"
+
+	url "net/url"
 )
 
-// ExecutorApi is an autogenerated mock type for the ExecutorApi type
+// nomadApiQuerierMock is an autogenerated mock type for the nomadApiQuerier type
-type ExecutorApi struct {
+type nomadApiQuerierMock struct {
 	mock.Mock
 }
 
 // DeleteRunner provides a mock function with given fields: runnerId
-func (_m *ExecutorApi) DeleteRunner(runnerId string) error {
+func (_m *nomadApiQuerierMock) DeleteRunner(runnerId string) error {
 	ret := _m.Called(runnerId)
 
 	var r0 error
@@ -27,7 +29,7 @@ func (_m *ExecutorApi) DeleteRunner(runnerId string) error {
 }
 
 // GetJobScale provides a mock function with given fields: jobId
-func (_m *ExecutorApi) GetJobScale(jobId string) (int, error) {
+func (_m *nomadApiQuerierMock) GetJobScale(jobId string) (int, error) {
 	ret := _m.Called(jobId)
 
 	var r0 int
@@ -48,7 +50,7 @@ func (_m *ExecutorApi) GetJobScale(jobId string) (int, error) {
 }
 
 // LoadJobList provides a mock function with given fields:
-func (_m *ExecutorApi) LoadJobList() ([]*api.JobListStub, error) {
+func (_m *nomadApiQuerierMock) LoadJobList() ([]*api.JobListStub, error) {
 	ret := _m.Called()
 
 	var r0 []*api.JobListStub
@@ -70,16 +72,44 @@ func (_m *ExecutorApi) LoadJobList() ([]*api.JobListStub, error) {
 	return r0, r1
 }
 
-// LoadRunners provides a mock function with given fields: jobId
+// SetJobScaling provides a mock function with given fields: jobId, count, reason
-func (_m *ExecutorApi) LoadAvailableRunners(jobId string) ([]string, error) {
+func (_m *nomadApiQuerierMock) SetJobScaling(jobId string, count int, reason string) error {
+	ret := _m.Called(jobId, count, reason)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, int, string) error); ok {
+		r0 = rf(jobId, count, reason)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// init provides a mock function with given fields: nomadURL
+func (_m *nomadApiQuerierMock) init(nomadURL *url.URL) error {
+	ret := _m.Called(nomadURL)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(*url.URL) error); ok {
+		r0 = rf(nomadURL)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// loadRunners provides a mock function with given fields: jobId
+func (_m *nomadApiQuerierMock) loadRunners(jobId string) ([]*api.AllocationListStub, error) {
 	ret := _m.Called(jobId)
 
-	var r0 []string
+	var r0 []*api.AllocationListStub
-	if rf, ok := ret.Get(0).(func(string) []string); ok {
+	if rf, ok := ret.Get(0).(func(string) []*api.AllocationListStub); ok {
 		r0 = rf(jobId)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).([]string)
+			r0 = ret.Get(0).([]*api.AllocationListStub)
 		}
 	}
 
@@ -92,17 +122,3 @@ func (_m *ExecutorApi) LoadAvailableRunners(jobId string) ([]string, error) {
 
 	return r0, r1
 }
-
-// SetJobScaling provides a mock function with given fields: jobId, count, reason
-func (_m *ExecutorApi) SetJobScaling(jobId string, count int, reason string) error {
-	ret := _m.Called(jobId, count, reason)
-
-	var r0 error
-	if rf, ok := ret.Get(0).(func(string, int, string) error); ok {
-		r0 = rf(jobId, count, reason)
-	} else {
-		r0 = ret.Error(0)
-	}
-
-	return r0
-}

nomad/nomad_test.go

@@ -0,0 +1,114 @@
+package nomad
+
+import (
+	"errors"
+	nomadApi "github.com/hashicorp/nomad/api"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/suite"
+	"testing"
+)
+
+func TestLoadAvailableRunnersTestSuite(t *testing.T) {
+	suite.Run(t, new(LoadAvailableRunnersTestSuite))
+}
+
+type LoadAvailableRunnersTestSuite struct {
+	suite.Suite
+	jobId                  string
+	mock                   *nomadApiQuerierMock
+	nomadApiClient         ApiClient
+	availableRunner        *nomadApi.AllocationListStub
+	anotherAvailableRunner *nomadApi.AllocationListStub
+	stoppedRunner          *nomadApi.AllocationListStub
+	stoppingRunner         *nomadApi.AllocationListStub
+}
+
+func (suite *LoadAvailableRunnersTestSuite) SetupTest() {
+	suite.jobId = "1d-0f-v3ry-sp3c14l-j0b"
+
+	suite.mock = &nomadApiQuerierMock{}
+	suite.nomadApiClient = ApiClient{nomadApiQuerier: suite.mock}
+
+	suite.availableRunner = &nomadApi.AllocationListStub{
+		ID:            "s0m3-r4nd0m-1d",
+		ClientStatus:  nomadApi.AllocClientStatusRunning,
+		DesiredStatus: nomadApi.AllocDesiredStatusRun,
+	}
+
+	suite.anotherAvailableRunner = &nomadApi.AllocationListStub{
+		ID:            "s0m3-s1m1l4r-1d",
+		ClientStatus:  nomadApi.AllocClientStatusRunning,
+		DesiredStatus: nomadApi.AllocDesiredStatusRun,
+	}
+
+	suite.stoppedRunner = &nomadApi.AllocationListStub{
+		ID:            "4n0th3r-1d",
+		ClientStatus:  nomadApi.AllocClientStatusComplete,
+		DesiredStatus: nomadApi.AllocDesiredStatusRun,
+	}
+
+	suite.stoppingRunner = &nomadApi.AllocationListStub{
+		ID:            "th1rd-1d",
+		ClientStatus:  nomadApi.AllocClientStatusRunning,
+		DesiredStatus: nomadApi.AllocDesiredStatusStop,
+	}
+}
+
+func (suite *LoadAvailableRunnersTestSuite) TestErrorOfUnderlyingApiCallIsPropagated() {
+	errorString := "api errored"
+	suite.mock.On("loadRunners", mock.AnythingOfType("string")).
+		Return(nil, errors.New(errorString))
+
+	returnedIds, err := suite.nomadApiClient.LoadAvailableRunners(suite.jobId)
+	suite.Nil(returnedIds)
+	suite.Error(err)
+}
+
+func (suite *LoadAvailableRunnersTestSuite) TestThrowsNoErrorWhenUnderlyingApiCallDoesnt() {
+	suite.mock.On("loadRunners", mock.AnythingOfType("string")).
+		Return([]*nomadApi.AllocationListStub{}, nil)
+
+	_, err := suite.nomadApiClient.LoadAvailableRunners(suite.jobId)
+	suite.NoError(err)
+}
+
+func (suite *LoadAvailableRunnersTestSuite) TestAvailableRunnerIsReturned() {
+	suite.mock.On("loadRunners", mock.AnythingOfType("string")).
+		Return([]*nomadApi.AllocationListStub{suite.availableRunner}, nil)
+
+	returnedIds, _ := suite.nomadApiClient.LoadAvailableRunners(suite.jobId)
+	suite.Len(returnedIds, 1)
+	suite.Equal(suite.availableRunner.ID, returnedIds[0])
+}
+
+func (suite *LoadAvailableRunnersTestSuite) TestStoppedRunnerIsNotReturned() {
+	suite.mock.On("loadRunners", mock.AnythingOfType("string")).
+		Return([]*nomadApi.AllocationListStub{suite.stoppedRunner}, nil)
+
+	returnedIds, _ := suite.nomadApiClient.LoadAvailableRunners(suite.jobId)
+	suite.Empty(returnedIds)
+}
+
+func (suite *LoadAvailableRunnersTestSuite) TestStoppingRunnerIsNotReturned() {
+	suite.mock.On("loadRunners", mock.AnythingOfType("string")).
+		Return([]*nomadApi.AllocationListStub{suite.stoppingRunner}, nil)
+
+	returnedIds, _ := suite.nomadApiClient.LoadAvailableRunners(suite.jobId)
+	suite.Empty(returnedIds)
+}
+
+func (suite *LoadAvailableRunnersTestSuite) TestReturnsAllAvailableRunners() {
+	runnersList := []*nomadApi.AllocationListStub{
+		suite.availableRunner,
+		suite.anotherAvailableRunner,
+		suite.stoppedRunner,
+		suite.stoppingRunner,
+	}
+	suite.mock.On("loadRunners", mock.AnythingOfType("string")).
+		Return(runnersList, nil)
+
+	returnedIds, _ := suite.nomadApiClient.LoadAvailableRunners(suite.jobId)
+	suite.Len(returnedIds, 2)
+	suite.Contains(returnedIds, suite.availableRunner.ID)
+	suite.Contains(returnedIds, suite.anotherAvailableRunner.ID)
+}