oss/forgejo-runner
oss/forgejo-runner
1
0
Fork 0
mirror of https://code.forgejo.org/forgejo/runner.git synced 2025-09-05 18:40:59 +00:00
Code Wiki Activity

fix: an error from a SKIPPED job does not transform it into a FAILURE (#896)

Browse source 
The result of a job can only be changed to FAILURE by the Close() function of the reporter under two conditions:

- If it is UNSPECIFIED
- If the context timeout

In all other cases it must be preserved. It is possible, for instance, for a job to be SKIPPED and be associated with an error message, e.g.

```
time="2025-08-20T21:02:07+02:00" level=trace msg="evaluating expression 'success()'" dryrun=false job="Test Action/job-1" jobID=job-1 matrix="map[]"
[Test Action/job-1] [DEBUG] evaluating expression 'success()'
time="2025-08-20T21:02:07+02:00" level=trace msg="expression 'success()' evaluated to 'true'" dryrun=false job="Test Action/job-1" jobID=job-1 matrix="map[]"
[Test Action/job-1] [DEBUG] expression 'success()' evaluated to 'true'
time="2025-08-20T21:02:07+02:00" level=trace msg="'runs-on' key not defined in Test Action/job-1" dryrun=false job="Test Action/job-1" jobID=job-1 matrix="map[]"
[Test Action/job-1] 'runs-on' key not defined in Test Action/job-1
[Test Action/job-1] [DEBUG] No steps found
```

Those errors show in the logs when a job is skipped, because the worflow is empty. But they are expected and to be ignored.

Refs forgejo/runner#895

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/896): <!--number 896 --><!--line 0 --><!--description Zml4OiBhbiBlcnJvciBmcm9tIGEgU0tJUFBFRCBqb2IgZG9lcyBub3QgdHJhbnNmb3JtIGl0IGludG8gYSBGQUlMVVJF-->fix: an error from a SKIPPED job does not transform it into a FAILURE<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/896
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This commit is contained in:
Earl Warren 2025-08-20 22:02:43 +00:00 • committed by earl-warren
parent 8f5470ad37
commit b8f8b0f674
No known key found for this signature in database
GPG key ID: F128CBE6AB3A7201
2 changed files with 48 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

16
internal/pkg/report/reporter.go
View file

@ -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 != "" {

70
internal/pkg/report/reporter_test.go
View file

@ -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)
})
}
}