diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go index a02f0e72..42afe8e0 100644 --- a/internal/pkg/report/reporter.go +++ b/internal/pkg/report/reporter.go @@ -244,26 +244,30 @@ const ( func (r *Reporter) Close(runErr error) error { r.closed = true + setStepCancel := func() { + for _, v := range r.state.Steps { + if v.Result == runnerv1.Result_RESULT_UNSPECIFIED { + v.Result = runnerv1.Result_RESULT_CANCELLED + } + } + } + r.stateMu.Lock() var lastWords string if errors.Is(runErr, context.DeadlineExceeded) { lastWords = closeTimeoutMessage r.state.Result = runnerv1.Result_RESULT_CANCELLED + setStepCancel() } else if r.state.Result == runnerv1.Result_RESULT_UNSPECIFIED { if runErr == nil { lastWords = closeCancelledMessage } else { lastWords = runErr.Error() } - for _, v := range r.state.Steps { - if v.Result == runnerv1.Result_RESULT_UNSPECIFIED { - v.Result = runnerv1.Result_RESULT_CANCELLED - } - } r.state.Result = runnerv1.Result_RESULT_FAILURE + setStepCancel() } else if runErr != nil { lastWords = runErr.Error() - r.state.Result = runnerv1.Result_RESULT_FAILURE } if lastWords != "" { diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go index 40af3e30..246d1699 100644 --- a/internal/pkg/report/reporter_test.go +++ b/internal/pkg/report/reporter_test.go @@ -410,7 +410,8 @@ func TestReporterReportLog(t *testing.T) { } func TestReporterClose(t *testing.T) { - mockReporterCloser := func(message *string, result *runnerv1.Result) *Reporter { + mockReporterCloser := func(t *testing.T, message *string, result *runnerv1.Result) *Reporter { + t.Helper() reporter, client, _ := mockReporter(t) if message != nil { client.On("UpdateLog", mock.Anything, mock.Anything).Return(func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) { @@ -435,46 +436,50 @@ func TestReporterClose(t *testing.T) { } for _, testCase := range []struct { - name string - err error - expectedMessage string - result runnerv1.Result - expectedResult runnerv1.Result + name string + err error + expectedMessage string + result runnerv1.Result + expectedStateResult runnerv1.Result + expectedStepStateResult runnerv1.Result }{ { - name: "ResultSuccessAndNilErrorIsResultSuccess", - err: nil, - expectedMessage: "", - result: runnerv1.Result_RESULT_SUCCESS, - expectedResult: runnerv1.Result_RESULT_SUCCESS, + name: "ResultSuccessAndNilErrorIsResultSuccess", + err: nil, + expectedMessage: "", + result: runnerv1.Result_RESULT_SUCCESS, + expectedStateResult: runnerv1.Result_RESULT_SUCCESS, }, { - name: "ResultUnspecifiedAndErrorIsResultFailure", - err: errors.New("ERROR_MESSAGE"), - expectedMessage: "ERROR_MESSAGE", - result: runnerv1.Result_RESULT_UNSPECIFIED, - expectedResult: runnerv1.Result_RESULT_FAILURE, + name: "ResultUnspecifiedAndErrorIsResultFailure", + err: errors.New("ERROR_MESSAGE"), + expectedMessage: "ERROR_MESSAGE", + result: runnerv1.Result_RESULT_UNSPECIFIED, + expectedStateResult: runnerv1.Result_RESULT_FAILURE, + expectedStepStateResult: runnerv1.Result_RESULT_CANCELLED, }, { - name: "ResultUnspecifiedAndNilErrorIsResultFailure", - err: nil, - expectedMessage: closeCancelledMessage, - result: runnerv1.Result_RESULT_UNSPECIFIED, - expectedResult: runnerv1.Result_RESULT_FAILURE, + name: "ResultUnspecifiedAndNilErrorIsResultFailure", + err: nil, + expectedMessage: closeCancelledMessage, + result: runnerv1.Result_RESULT_UNSPECIFIED, + expectedStateResult: runnerv1.Result_RESULT_FAILURE, + expectedStepStateResult: runnerv1.Result_RESULT_CANCELLED, }, { - name: "ResultSuccessAndErrorIsResultFailure", - err: errors.New("ERROR_MESSAGE"), - expectedMessage: "ERROR_MESSAGE", - result: runnerv1.Result_RESULT_SUCCESS, - expectedResult: runnerv1.Result_RESULT_FAILURE, + name: "ResultSkippedAndErrorIsResultSkipped", + err: errors.New("ERROR_MESSAGE"), + expectedMessage: "ERROR_MESSAGE", + result: runnerv1.Result_RESULT_SKIPPED, + expectedStateResult: runnerv1.Result_RESULT_SKIPPED, }, { - name: "Timeout", - err: context.DeadlineExceeded, - expectedMessage: closeTimeoutMessage, - result: runnerv1.Result_RESULT_UNSPECIFIED, - expectedResult: runnerv1.Result_RESULT_CANCELLED, + name: "Timeout", + err: context.DeadlineExceeded, + expectedMessage: closeTimeoutMessage, + result: runnerv1.Result_RESULT_UNSPECIFIED, + expectedStateResult: runnerv1.Result_RESULT_CANCELLED, + expectedStepStateResult: runnerv1.Result_RESULT_CANCELLED, }, } { t.Run(testCase.name, func(t *testing.T) { @@ -482,7 +487,7 @@ func TestReporterClose(t *testing.T) { if testCase.expectedMessage != "" { message = &testCase.expectedMessage } - reporter := mockReporterCloser(message, &testCase.expectedResult) + reporter := mockReporterCloser(t, message, &testCase.expectedStateResult) // cancel() verifies Close can operate after the context is cancelled // because it uses the daemon context instead @@ -494,6 +499,7 @@ func TestReporterClose(t *testing.T) { }, } require.NoError(t, reporter.Close(testCase.err)) + assert.Equal(t, testCase.expectedStepStateResult, reporter.state.Steps[0].Result) }) } }