diff --git a/README.md b/README.md index decf94e..8dde451 100644 --- a/README.md +++ b/README.md @@ -136,8 +136,8 @@ For example, for an interface called `ExecutorApi` in the package `nomad`, you m ```bash mockery \ --name=ExecutorApi \ ---structname=ExecutorApiMock \ ---filename=ExecutorApiMock.go \ +--structname=ExecutorAPIMock \ +--filename=ExecutorAPIMock.go \ --inpackage ``` diff --git a/api/websocket_test.go b/api/websocket_test.go index 6502797..5b6a362 100644 --- a/api/websocket_test.go +++ b/api/websocket_test.go @@ -34,7 +34,7 @@ type WebSocketTestSuite struct { router *mux.Router executionId runner.ExecutionId runner runner.Runner - apiMock *nomad.ExecutorApiMock + apiMock *nomad.ExecutorAPIMock server *httptest.Server } @@ -307,8 +307,8 @@ func TestRawToCodeOceanWriter(t *testing.T) { // --- Test suite specific test helpers --- -func newNomadAllocationWithMockedApiClient(runnerId string) (r runner.Runner, mock *nomad.ExecutorApiMock) { - mock = &nomad.ExecutorApiMock{} +func newNomadAllocationWithMockedApiClient(runnerId string) (r runner.Runner, mock *nomad.ExecutorAPIMock) { + mock = &nomad.ExecutorAPIMock{} r = runner.NewNomadAllocation(runnerId, mock) return } @@ -335,7 +335,7 @@ func (suite *WebSocketTestSuite) webSocketUrl(scheme, runnerId string, execution var executionRequestLs = dto.ExecutionRequest{Command: "ls"} // mockApiExecuteLs mocks the ExecuteCommand of an ExecutorApi to act as if 'ls existing-file non-existing-file' was executed. -func mockApiExecuteLs(api *nomad.ExecutorApiMock) { +func mockApiExecuteLs(api *nomad.ExecutorAPIMock) { helpers.MockApiExecute(api, &executionRequestLs, func(_ string, _ context.Context, _ []string, _ bool, _ io.Reader, stdout, stderr io.Writer) (int, error) { _, _ = stdout.Write([]byte("existing-file\n")) @@ -347,7 +347,7 @@ func mockApiExecuteLs(api *nomad.ExecutorApiMock) { var executionRequestHead = dto.ExecutionRequest{Command: "head -n 1"} // mockApiExecuteHead mocks the ExecuteCommand of an ExecutorApi to act as if 'head -n 1' was executed. -func mockApiExecuteHead(api *nomad.ExecutorApiMock) { +func mockApiExecuteHead(api *nomad.ExecutorAPIMock) { helpers.MockApiExecute(api, &executionRequestHead, func(_ string, _ context.Context, _ []string, _ bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { scanner := bufio.NewScanner(stdin) @@ -362,7 +362,7 @@ func mockApiExecuteHead(api *nomad.ExecutorApiMock) { var executionRequestSleep = dto.ExecutionRequest{Command: "sleep infinity"} // mockApiExecuteSleep mocks the ExecuteCommand method of an ExecutorAPI to sleep until the execution is canceled. -func mockApiExecuteSleep(api *nomad.ExecutorApiMock) <-chan bool { +func mockApiExecuteSleep(api *nomad.ExecutorAPIMock) <-chan bool { canceled := make(chan bool, 1) helpers.MockApiExecute(api, &executionRequestSleep, func(_ string, ctx context.Context, _ []string, _ bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) (int, error) { @@ -376,7 +376,7 @@ func mockApiExecuteSleep(api *nomad.ExecutorApiMock) <-chan bool { var executionRequestError = dto.ExecutionRequest{Command: "error"} // mockApiExecuteError mocks the ExecuteCommand method of an ExecutorApi to return an error. -func mockApiExecuteError(api *nomad.ExecutorApiMock) { +func mockApiExecuteError(api *nomad.ExecutorAPIMock) { helpers.MockApiExecute(api, &executionRequestError, func(_ string, _ context.Context, _ []string, _ bool, _ io.Reader, _, _ io.Writer) (int, error) { return 0, errors.New("intended error") @@ -386,7 +386,7 @@ func mockApiExecuteError(api *nomad.ExecutorApiMock) { var executionRequestExitNonZero = dto.ExecutionRequest{Command: "exit 42"} // mockApiExecuteExitNonZero mocks the ExecuteCommand method of an ExecutorApi to exit with exit status 42. -func mockApiExecuteExitNonZero(api *nomad.ExecutorApiMock) { +func mockApiExecuteExitNonZero(api *nomad.ExecutorAPIMock) { helpers.MockApiExecute(api, &executionRequestExitNonZero, func(_ string, _ context.Context, _ []string, _ bool, _ io.Reader, _, _ io.Writer) (int, error) { return 42, nil diff --git a/environment/job_test.go b/environment/job_test.go index 8223e17..cce0d25 100644 --- a/environment/job_test.go +++ b/environment/job_test.go @@ -261,7 +261,7 @@ func TestCreateJobSetsAllGivenArguments(t *testing.T) { } func TestRegisterJobWhenNomadJobRegistrationFails(t *testing.T) { - apiMock := nomad.ExecutorApiMock{} + apiMock := nomad.ExecutorAPIMock{} expectedErr := errors.New("test error") apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return("", expectedErr) @@ -278,7 +278,7 @@ func TestRegisterJobWhenNomadJobRegistrationFails(t *testing.T) { } func TestRegisterJobSucceedsWhenMonitoringEvaluationSucceeds(t *testing.T) { - apiMock := nomad.ExecutorApiMock{} + apiMock := nomad.ExecutorAPIMock{} evaluationID := "id" apiMock.On("RegisterNomadJob", mock.AnythingOfType("*api.Job")).Return(evaluationID, nil) @@ -295,7 +295,7 @@ func TestRegisterJobSucceedsWhenMonitoringEvaluationSucceeds(t *testing.T) { } func TestRegisterJobReturnsErrorWhenMonitoringEvaluationFails(t *testing.T) { - apiMock := nomad.ExecutorApiMock{} + apiMock := nomad.ExecutorAPIMock{} evaluationID := "id" expectedErr := errors.New("test error") diff --git a/environment/manager_test.go b/environment/manager_test.go index 1714aa3..d86335e 100644 --- a/environment/manager_test.go +++ b/environment/manager_test.go @@ -16,7 +16,7 @@ import ( type CreateOrUpdateTestSuite struct { suite.Suite runnerManagerMock runner.ManagerMock - apiMock nomad.ExecutorApiMock + apiMock nomad.ExecutorAPIMock registerNomadJobMockCall *mock.Call request dto.ExecutionEnvironmentRequest manager *NomadEnvironmentManager @@ -28,7 +28,7 @@ func TestCreateOrUpdateTestSuite(t *testing.T) { func (s *CreateOrUpdateTestSuite) SetupTest() { s.runnerManagerMock = runner.ManagerMock{} - s.apiMock = nomad.ExecutorApiMock{} + s.apiMock = nomad.ExecutorAPIMock{} s.request = dto.ExecutionEnvironmentRequest{ PrewarmingPoolSize: 10, CPULimit: 20, diff --git a/nomad/nomad_test.go b/nomad/nomad_test.go index 5c0d47f..f971b11 100644 --- a/nomad/nomad_test.go +++ b/nomad/nomad_test.go @@ -1,6 +1,7 @@ package nomad import ( + "bytes" "context" "errors" "fmt" @@ -11,8 +12,12 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "gitlab.hpi.de/codeocean/codemoon/poseidon/config" "gitlab.hpi.de/codeocean/codemoon/poseidon/tests" + "io" "net/url" + "regexp" + "strings" "testing" "time" ) @@ -617,3 +622,139 @@ func createAllocation(modifyTime int64, clientStatus, desiredStatus string) *nom func createRecentAllocation(clientStatus, desiredStatus string) *nomadApi.Allocation { return createAllocation(time.Now().Add(time.Minute).UnixNano(), clientStatus, desiredStatus) } + +func TestExecuteCommandTestSuite(t *testing.T) { + suite.Run(t, new(ExecuteCommandTestSuite)) +} + +type ExecuteCommandTestSuite struct { + suite.Suite + allocationID string + ctx context.Context + testCommand string + testCommandArray []string + expectedStdout string + expectedStderr string + apiMock *apiQuerierMock + nomadAPIClient APIClient +} + +func (s *ExecuteCommandTestSuite) SetupTest() { + s.allocationID = "test-allocation-id" + s.ctx = context.Background() + s.testCommand = "echo 'do nothing'" + s.testCommandArray = []string{"sh", "-c", s.testCommand} + s.expectedStdout = "stdout" + s.expectedStderr = "stderr" + s.apiMock = &apiQuerierMock{} + s.nomadAPIClient = APIClient{apiQuerier: s.apiMock} +} + +const withTTY = true + +func (s *ExecuteCommandTestSuite) TestWithSeparateStderr() { + config.Config.Server.InteractiveStderr = true + commandExitCode := 42 + stderrExitCode := 1 + + var stdout, stderr bytes.Buffer + var calledStdoutCommand, calledStderrCommand []string + + // mock regular call + s.mockExecute(s.testCommandArray, commandExitCode, nil, func(args mock.Arguments) { + var ok bool + calledStdoutCommand, ok = args.Get(2).([]string) + s.Require().True(ok) + writer, ok := args.Get(5).(io.Writer) + s.Require().True(ok) + _, err := writer.Write([]byte(s.expectedStdout)) + s.Require().NoError(err) + }) + // mock stderr call + s.mockExecute(mock.AnythingOfType("[]string"), stderrExitCode, nil, func(args mock.Arguments) { + var ok bool + calledStderrCommand, ok = args.Get(2).([]string) + s.Require().True(ok) + writer, ok := args.Get(5).(io.Writer) + s.Require().True(ok) + _, err := writer.Write([]byte(s.expectedStderr)) + s.Require().NoError(err) + }) + + exitCode, err := s.nomadAPIClient. + ExecuteCommand(s.allocationID, s.ctx, s.testCommandArray, withTTY, nullReader{}, &stdout, &stderr) + s.Require().NoError(err) + + s.apiMock.AssertNumberOfCalls(s.T(), "Execute", 2) + s.Equal(commandExitCode, exitCode) + + s.Run("should wrap command in stderr wrapper", func() { + s.Require().NotNil(calledStdoutCommand) + stdoutFifoRegexp := strings.ReplaceAll(regexp.QuoteMeta(stderrWrapperCommandFormat), "%d", "\\d*") + stdoutFifoRegexp = fmt.Sprintf(stdoutFifoRegexp, s.testCommand) + s.Regexp(stdoutFifoRegexp, calledStdoutCommand[len(calledStdoutCommand)-1]) + }) + + s.Run("should call correct stderr command", func() { + s.Require().NotNil(calledStderrCommand) + stderrFifoRegexp := strings.ReplaceAll(regexp.QuoteMeta(stderrFifoCommandFormat), "%d", "\\d*") + s.Regexp(stderrFifoRegexp, calledStderrCommand[len(calledStderrCommand)-1]) + }) + + s.Run("should return correct output", func() { + s.Equal(s.expectedStdout, stdout.String()) + s.Equal(s.expectedStderr, stderr.String()) + }) +} + +func (s *ExecuteCommandTestSuite) TestWithSeparateStderrReturnsCommandError() { + config.Config.Server.InteractiveStderr = true + s.mockExecute(s.testCommandArray, 1, tests.ErrDefault, func(args mock.Arguments) {}) + s.mockExecute(mock.AnythingOfType("[]string"), 1, nil, func(args mock.Arguments) {}) + _, err := s.nomadAPIClient. + ExecuteCommand(s.allocationID, s.ctx, s.testCommandArray, withTTY, nullReader{}, io.Discard, io.Discard) + s.Equal(tests.ErrDefault, err) +} + +func (s *ExecuteCommandTestSuite) TestWithoutSeparateStderr() { + config.Config.Server.InteractiveStderr = false + var stdout, stderr bytes.Buffer + commandExitCode := 42 + + // mock regular call + s.mockExecute(s.testCommandArray, commandExitCode, nil, func(args mock.Arguments) { + stdout, ok := args.Get(5).(io.Writer) + s.Require().True(ok) + _, err := stdout.Write([]byte(s.expectedStdout)) + s.Require().NoError(err) + stderr, ok := args.Get(6).(io.Writer) + s.Require().True(ok) + _, err = stderr.Write([]byte(s.expectedStderr)) + s.Require().NoError(err) + }) + + exitCode, err := s.nomadAPIClient. + ExecuteCommand(s.allocationID, s.ctx, s.testCommandArray, withTTY, nullReader{}, &stdout, &stderr) + s.Require().NoError(err) + + s.apiMock.AssertNumberOfCalls(s.T(), "Execute", 1) + s.Equal(commandExitCode, exitCode) + s.Equal(s.expectedStdout, stdout.String()) + s.Equal(s.expectedStderr, stderr.String()) +} + +func (s *ExecuteCommandTestSuite) TestWithoutSeparateStderrReturnsCommandError() { + config.Config.Server.InteractiveStderr = false + s.mockExecute(s.testCommandArray, 1, tests.ErrDefault, func(args mock.Arguments) {}) + _, err := s.nomadAPIClient. + ExecuteCommand(s.allocationID, s.ctx, s.testCommandArray, withTTY, nullReader{}, io.Discard, io.Discard) + s.Equal(tests.ErrDefault, err) +} + +func (s *ExecuteCommandTestSuite) mockExecute(command interface{}, exitCode int, + err error, runFunc func(arguments mock.Arguments)) { + s.apiMock.On("Execute", s.allocationID, s.ctx, command, withTTY, + mock.Anything, mock.Anything, mock.Anything). + Run(runFunc). + Return(exitCode, err) +} diff --git a/runner/manager_test.go b/runner/manager_test.go index 0e1a000..3595ca9 100644 --- a/runner/manager_test.go +++ b/runner/manager_test.go @@ -25,13 +25,13 @@ func TestGetNextRunnerTestSuite(t *testing.T) { type ManagerTestSuite struct { suite.Suite - apiMock *nomad.ExecutorApiMock + apiMock *nomad.ExecutorAPIMock nomadRunnerManager *NomadRunnerManager exerciseRunner Runner } func (s *ManagerTestSuite) SetupTest() { - s.apiMock = &nomad.ExecutorApiMock{} + s.apiMock = &nomad.ExecutorAPIMock{} // Instantly closed context to manually start the update process in some cases ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -42,7 +42,7 @@ func (s *ManagerTestSuite) SetupTest() { s.registerDefaultEnvironment() } -func mockRunnerQueries(apiMock *nomad.ExecutorApiMock, returnedRunnerIds []string) { +func mockRunnerQueries(apiMock *nomad.ExecutorAPIMock, returnedRunnerIds []string) { // reset expected calls to allow new mocked return values apiMock.ExpectedCalls = []*mock.Call{} call := apiMock.On("WatchAllocations", mock.Anything, mock.Anything, mock.Anything) @@ -273,7 +273,7 @@ func (s *ManagerTestSuite) TestUpdateRunnersRemovesIdleAndUsedRunner() { s.False(ok) } -func modifyMockedCall(apiMock *nomad.ExecutorApiMock, method string, modifier func(call *mock.Call)) { +func modifyMockedCall(apiMock *nomad.ExecutorAPIMock, method string, modifier func(call *mock.Call)) { for _, c := range apiMock.ExpectedCalls { if c.Method == method { modifier(c) diff --git a/runner/runner_test.go b/runner/runner_test.go index 986dee1..46bf074 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -73,7 +73,7 @@ func TestFromContextReturnsIsNotOkWhenContextHasNoRunner(t *testing.T) { } func TestExecuteCallsAPI(t *testing.T) { - apiMock := &nomad.ExecutorApiMock{} + 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) @@ -106,8 +106,8 @@ func TestExecuteReturnsAfterTimeout(t *testing.T) { } } -func newApiMockWithTimeLimitHandling() (apiMock *nomad.ExecutorApiMock) { - apiMock = &nomad.ExecutorApiMock{} +func newApiMockWithTimeLimitHandling() (apiMock *nomad.ExecutorAPIMock) { + apiMock = &nomad.ExecutorAPIMock{} apiMock. On("ExecuteCommand", mock.Anything, mock.Anything, mock.Anything, true, mock.Anything, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { @@ -125,14 +125,14 @@ func TestUpdateFileSystemTestSuite(t *testing.T) { type UpdateFileSystemTestSuite struct { suite.Suite runner *NomadAllocation - apiMock *nomad.ExecutorApiMock + apiMock *nomad.ExecutorAPIMock mockedExecuteCommandCall *mock.Call command []string stdin *bytes.Buffer } func (s *UpdateFileSystemTestSuite) SetupTest() { - s.apiMock = &nomad.ExecutorApiMock{} + s.apiMock = &nomad.ExecutorAPIMock{} 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). Run(func(args mock.Arguments) { diff --git a/tests/helpers/test_helpers.go b/tests/helpers/test_helpers.go index 820c7f0..4f7ed9a 100644 --- a/tests/helpers/test_helpers.go +++ b/tests/helpers/test_helpers.go @@ -114,7 +114,7 @@ func StartTLSServer(t *testing.T, router *mux.Router) (server *httptest.Server, // MockApiExecute mocks the ExecuteCommand method of an ExecutorApi to call the given method run when the command // corresponding to the given ExecutionRequest is called. -func MockApiExecute(api *nomad.ExecutorApiMock, request *dto.ExecutionRequest, +func MockApiExecute(api *nomad.ExecutorAPIMock, request *dto.ExecutionRequest, run func(runnerId string, ctx context.Context, command []string, tty bool, stdin io.Reader, stdout, stderr io.Writer) (int, error)) { call := api.On("ExecuteCommand", mock.AnythingOfType("string"),