diff options
author | Jason Song <i@wolfogre.com> | 2023-05-06 11:00:52 +0200 |
---|---|---|
committer | Jason Song <i@wolfogre.com> | 2023-05-06 11:00:52 +0200 |
commit | de4160b0231dcb0678fb454dc740535b9c23df59 (patch) | |
tree | ae48ea1690de19685965fd5963621aad20aee6d9 /internal | |
parent | fix `--event` option logic for `exec` (#175) (diff) | |
download | forgejo-runner-de4160b0231dcb0678fb454dc740535b9c23df59.tar.xz forgejo-runner-de4160b0231dcb0678fb454dc740535b9c23df59.zip |
Skip counting log length when parseLogRow return nil (#176)
Fix:
![image](/attachments/93e29bc0-3599-4f7e-8b90-512562a5d711)
Regression of #149, `LogLength` could be incorrect.
It may be related to https://github.com/go-gitea/gitea/issues/24458
Reviewed-on: https://gitea.com/gitea/act_runner/pulls/176
Reviewed-by: Zettat123 <zettat123@noreply.gitea.io>
Reviewed-by: wxiaoguang <wxiaoguang@noreply.gitea.io>
Diffstat (limited to 'internal')
-rw-r--r-- | internal/pkg/client/client.go | 2 | ||||
-rw-r--r-- | internal/pkg/client/mocks/Client.go | 193 | ||||
-rw-r--r-- | internal/pkg/report/reporter.go | 10 | ||||
-rw-r--r-- | internal/pkg/report/reporter_test.go | 51 |
4 files changed, 251 insertions, 5 deletions
diff --git a/internal/pkg/client/client.go b/internal/pkg/client/client.go index b9bda52..57f91ad 100644 --- a/internal/pkg/client/client.go +++ b/internal/pkg/client/client.go @@ -9,6 +9,8 @@ import ( ) // A Client manages communication with the runner. +// +//go:generate mockery --name Client type Client interface { pingv1connect.PingServiceClient runnerv1connect.RunnerServiceClient diff --git a/internal/pkg/client/mocks/Client.go b/internal/pkg/client/mocks/Client.go new file mode 100644 index 0000000..a689c54 --- /dev/null +++ b/internal/pkg/client/mocks/Client.go @@ -0,0 +1,193 @@ +// Code generated by mockery v2.26.1. DO NOT EDIT. + +package mocks + +import ( + context "context" + + connect "github.com/bufbuild/connect-go" + + mock "github.com/stretchr/testify/mock" + + pingv1 "code.gitea.io/actions-proto-go/ping/v1" + + runnerv1 "code.gitea.io/actions-proto-go/runner/v1" +) + +// Client is an autogenerated mock type for the Client type +type Client struct { + mock.Mock +} + +// Address provides a mock function with given fields: +func (_m *Client) Address() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// FetchTask provides a mock function with given fields: _a0, _a1 +func (_m *Client) FetchTask(_a0 context.Context, _a1 *connect.Request[runnerv1.FetchTaskRequest]) (*connect.Response[runnerv1.FetchTaskResponse], error) { + ret := _m.Called(_a0, _a1) + + var r0 *connect.Response[runnerv1.FetchTaskResponse] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) (*connect.Response[runnerv1.FetchTaskResponse], error)); ok { + return rf(_a0, _a1) + } + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) *connect.Response[runnerv1.FetchTaskResponse]); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connect.Response[runnerv1.FetchTaskResponse]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Insecure provides a mock function with given fields: +func (_m *Client) Insecure() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// Ping provides a mock function with given fields: _a0, _a1 +func (_m *Client) Ping(_a0 context.Context, _a1 *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error) { + ret := _m.Called(_a0, _a1) + + var r0 *connect.Response[pingv1.PingResponse] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error)); ok { + return rf(_a0, _a1) + } + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[pingv1.PingRequest]) *connect.Response[pingv1.PingResponse]); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connect.Response[pingv1.PingResponse]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[pingv1.PingRequest]) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Register provides a mock function with given fields: _a0, _a1 +func (_m *Client) Register(_a0 context.Context, _a1 *connect.Request[runnerv1.RegisterRequest]) (*connect.Response[runnerv1.RegisterResponse], error) { + ret := _m.Called(_a0, _a1) + + var r0 *connect.Response[runnerv1.RegisterResponse] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.RegisterRequest]) (*connect.Response[runnerv1.RegisterResponse], error)); ok { + return rf(_a0, _a1) + } + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.RegisterRequest]) *connect.Response[runnerv1.RegisterResponse]); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connect.Response[runnerv1.RegisterResponse]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.RegisterRequest]) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// UpdateLog provides a mock function with given fields: _a0, _a1 +func (_m *Client) UpdateLog(_a0 context.Context, _a1 *connect.Request[runnerv1.UpdateLogRequest]) (*connect.Response[runnerv1.UpdateLogResponse], error) { + ret := _m.Called(_a0, _a1) + + var r0 *connect.Response[runnerv1.UpdateLogResponse] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) (*connect.Response[runnerv1.UpdateLogResponse], error)); ok { + return rf(_a0, _a1) + } + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) *connect.Response[runnerv1.UpdateLogResponse]); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connect.Response[runnerv1.UpdateLogResponse]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// UpdateTask provides a mock function with given fields: _a0, _a1 +func (_m *Client) UpdateTask(_a0 context.Context, _a1 *connect.Request[runnerv1.UpdateTaskRequest]) (*connect.Response[runnerv1.UpdateTaskResponse], error) { + ret := _m.Called(_a0, _a1) + + var r0 *connect.Response[runnerv1.UpdateTaskResponse] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) (*connect.Response[runnerv1.UpdateTaskResponse], error)); ok { + return rf(_a0, _a1) + } + if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) *connect.Response[runnerv1.UpdateTaskResponse]); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connect.Response[runnerv1.UpdateTaskResponse]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type mockConstructorTestingTNewClient interface { + mock.TestingT + Cleanup(func()) +} + +// NewClient creates a new instance of Client. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewClient(t mockConstructorTestingTNewClient) *Client { + mock := &Client{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go index 10aa7e6..7e9a2d5 100644 --- a/internal/pkg/report/reporter.go +++ b/internal/pkg/report/reporter.go @@ -139,11 +139,13 @@ func (r *Reporter) Fire(entry *log.Entry) error { } if v, ok := entry.Data["raw_output"]; ok { if rawOutput, ok := v.(bool); ok && rawOutput { - if step.LogLength == 0 { - step.LogIndex = int64(r.logOffset + len(r.logRows)) + if row := r.parseLogRow(entry); row != nil { + if step.LogLength == 0 { + step.LogIndex = int64(r.logOffset + len(r.logRows)) + } + step.LogLength++ + r.logRows = append(r.logRows, row) } - step.LogLength++ - r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry)) } } else if !r.duringSteps() { r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry)) diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go index 6682a33..d3d4c12 100644 --- a/internal/pkg/report/reporter_test.go +++ b/internal/pkg/report/reporter_test.go @@ -4,11 +4,19 @@ package report import ( + "context" "strings" "testing" + runnerv1 "code.gitea.io/actions-proto-go/runner/v1" + connect_go "github.com/bufbuild/connect-go" log "github.com/sirupsen/logrus" - "gotest.tools/v3/assert" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" + + "gitea.com/gitea/act_runner/internal/pkg/client/mocks" ) func TestReporter_parseLogRow(t *testing.T) { @@ -146,3 +154,44 @@ func TestReporter_parseLogRow(t *testing.T) { }) } } + +func TestReporter_Fire(t *testing.T) { + t.Run("ignore command lines", func(t *testing.T) { + client := mocks.NewClient(t) + client.On("UpdateLog", mock.Anything, mock.Anything).Return(func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) { + t.Logf("Received UpdateLog: %s", req.Msg.String()) + return connect_go.NewResponse(&runnerv1.UpdateLogResponse{ + AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)), + }), nil + }) + client.On("UpdateTask", mock.Anything, mock.Anything).Return(func(_ context.Context, req *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) { + t.Logf("Received UpdateTask: %s", req.Msg.String()) + return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil + }) + ctx, cancel := context.WithCancel(context.Background()) + taskCtx, err := structpb.NewStruct(map[string]interface{}{}) + require.NoError(t, err) + reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{ + Context: taskCtx, + }) + defer func() { + assert.NoError(t, reporter.Close("")) + }() + reporter.ResetSteps(5) + + dataStep0 := map[string]interface{}{ + "stage": "Main", + "stepNumber": 0, + "raw_output": true, + } + + assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) + assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) + assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) + assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) + assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) + assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) + + assert.Equal(t, int64(3), reporter.state.Steps[0].LogLength) + }) +} |