invoke interpolateResults before setJobResult

act/runner/job_executor.go

@@ -104,7 +104,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 		}
 	}
 
-	postExecutor = postExecutor.Finally(func(ctx context.Context) error {
+	setJobResults := func(ctx context.Context) error {
 		jobError := common.JobError(ctx)
 
 		// Fresh context to ensure job result output works even if prev. context was a cancelled job
@@ -113,8 +113,12 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 		setJobResult(ctx, info, rc, jobError == nil)
 		setJobOutputs(ctx, rc)
 
+		return nil
+	}
+
+	cleanupJob := func(_ctx context.Context) error {
 		var err error
-		{
+
 		// Separate timeout for cleanup tasks; logger is cleared so that cleanup logs go to runner, not job
 		ctx, cancel := context.WithTimeout(context.Background(), cleanupTimeout)
 		defer cancel()
@@ -132,9 +136,9 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 				logger.Errorf("Error while cleaning network %s: %v", networkName, err)
 			}
 		}
-		}
+
 		return err
-	})
+	}
 
 	pipeline := make([]common.Executor, 0)
 	pipeline = append(pipeline, preSteps...)
@@ -152,6 +156,8 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 			return postExecutor(ctx)
 		}).
 		Finally(info.interpolateOutputs()).
+		Finally(setJobResults).
+		Finally(cleanupJob).
 		Finally(info.closeContainer()))
 }
 

act/runner/job_executor_test.go

@@ -12,10 +12,14 @@ import (
 	"code.forgejo.org/forgejo/runner/v11/act/common"
 	"code.forgejo.org/forgejo/runner/v11/act/container"
 	"code.forgejo.org/forgejo/runner/v11/act/model"
+	"code.forgejo.org/forgejo/runner/v11/act/runner/mocks"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
 
+//go:generate mockery --srcpkg=github.com/sirupsen/logrus --name=FieldLogger
+
 func TestJobExecutor(t *testing.T) {
 	tables := []TestJobFileInfo{
 		{workdir, "uses-and-run-in-one-step", "push", "Invalid run/uses syntax for job:test step:Test", platforms, secrets},
@@ -127,8 +131,9 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 			executedSteps: []string{
 				"startContainer",
 				"step1",
-				"stopContainer",
 				"interpolateOutputs",
+				"setJobResults",
+				"stopContainer",
 				"closeContainer",
 			},
 			result:   "success",
@@ -144,8 +149,9 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 			executedSteps: []string{
 				"startContainer",
 				"step1",
-				"stopContainer",
 				"interpolateOutputs",
+				"setJobResults",
+				"stopContainer",
 				"closeContainer",
 			},
 			result:   "failure",
@@ -162,8 +168,9 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 				"startContainer",
 				"pre1",
 				"step1",
-				"stopContainer",
 				"interpolateOutputs",
+				"setJobResults",
+				"stopContainer",
 				"closeContainer",
 			},
 			result:   "success",
@@ -180,8 +187,9 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 				"startContainer",
 				"step1",
 				"post1",
-				"stopContainer",
 				"interpolateOutputs",
+				"setJobResults",
+				"stopContainer",
 				"closeContainer",
 			},
 			result:   "success",
@@ -199,8 +207,9 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 				"pre1",
 				"step1",
 				"post1",
-				"stopContainer",
 				"interpolateOutputs",
+				"setJobResults",
+				"stopContainer",
 				"closeContainer",
 			},
 			result:   "success",
@@ -229,8 +238,9 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 				"step3",
 				"post3",
 				"post2",
-				"stopContainer",
 				"interpolateOutputs",
+				"setJobResults",
+				"stopContainer",
 				"closeContainer",
 			},
 			result:   "success",
@@ -246,7 +256,19 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			fmt.Printf("::group::%s\n", tt.name)
 
-			ctx := common.WithJobErrorContainer(t.Context())
+			executorOrder := make([]string, 0)
+
+			mockLogger := mocks.NewFieldLogger(t)
+			mockLogger.On("Debugf", mock.Anything, mock.Anything).Return(0).Maybe()
+			mockLogger.On("WithField", "jobResult", mock.Anything).
+				Run(func(args mock.Arguments) {
+					executorOrder = append(executorOrder, "setJobResults")
+				}).
+				Return(&logrus.Entry{Logger: &logrus.Logger{}}).Maybe()
+			mockLogger.On("WithField", mock.Anything, mock.Anything).Return(&logrus.Entry{Logger: &logrus.Logger{}}).Maybe()
+			mockLogger.On("WithFields", mock.Anything).Return(&logrus.Entry{Logger: &logrus.Logger{}}).Maybe()
+
+			ctx := common.WithLogger(common.WithJobErrorContainer(t.Context()), mockLogger)
 			jim := &jobInfoMock{}
 			sfm := &stepFactoryMock{}
 			rc := &RunContext{
@@ -262,7 +284,6 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 				Config: &Config{},
 			}
 			rc.ExprEval = rc.NewExpressionEvaluator(ctx)
-			executorOrder := make([]string, 0)
 
 			jim.On("steps").Return(tt.steps)
 

act/runner/mocks/FieldLogger.go

@@ -0,0 +1,264 @@
+// Code generated by mockery v2.53.5. DO NOT EDIT.
+
+package mocks
+
+import (
+	logrus "github.com/sirupsen/logrus"
+	mock "github.com/stretchr/testify/mock"
+)
+
+// FieldLogger is an autogenerated mock type for the FieldLogger type
+type FieldLogger struct {
+	mock.Mock
+}
+
+// Debug provides a mock function with given fields: args
+func (_m *FieldLogger) Debug(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Debugf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Debugf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Debugln provides a mock function with given fields: args
+func (_m *FieldLogger) Debugln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Error provides a mock function with given fields: args
+func (_m *FieldLogger) Error(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Errorf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Errorf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Errorln provides a mock function with given fields: args
+func (_m *FieldLogger) Errorln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Fatal provides a mock function with given fields: args
+func (_m *FieldLogger) Fatal(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Fatalf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Fatalf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Fatalln provides a mock function with given fields: args
+func (_m *FieldLogger) Fatalln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Info provides a mock function with given fields: args
+func (_m *FieldLogger) Info(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Infof provides a mock function with given fields: format, args
+func (_m *FieldLogger) Infof(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Infoln provides a mock function with given fields: args
+func (_m *FieldLogger) Infoln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Panic provides a mock function with given fields: args
+func (_m *FieldLogger) Panic(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Panicf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Panicf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Panicln provides a mock function with given fields: args
+func (_m *FieldLogger) Panicln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Print provides a mock function with given fields: args
+func (_m *FieldLogger) Print(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Printf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Printf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Println provides a mock function with given fields: args
+func (_m *FieldLogger) Println(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Warn provides a mock function with given fields: args
+func (_m *FieldLogger) Warn(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Warnf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Warnf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Warning provides a mock function with given fields: args
+func (_m *FieldLogger) Warning(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Warningf provides a mock function with given fields: format, args
+func (_m *FieldLogger) Warningf(format string, args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, format)
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Warningln provides a mock function with given fields: args
+func (_m *FieldLogger) Warningln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// Warnln provides a mock function with given fields: args
+func (_m *FieldLogger) Warnln(args ...interface{}) {
+	var _ca []interface{}
+	_ca = append(_ca, args...)
+	_m.Called(_ca...)
+}
+
+// WithError provides a mock function with given fields: err
+func (_m *FieldLogger) WithError(err error) *logrus.Entry {
+	ret := _m.Called(err)
+
+	if len(ret) == 0 {
+		panic("no return value specified for WithError")
+	}
+
+	var r0 *logrus.Entry
+	if rf, ok := ret.Get(0).(func(error) *logrus.Entry); ok {
+		r0 = rf(err)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*logrus.Entry)
+		}
+	}
+
+	return r0
+}
+
+// WithField provides a mock function with given fields: key, value
+func (_m *FieldLogger) WithField(key string, value interface{}) *logrus.Entry {
+	ret := _m.Called(key, value)
+
+	if len(ret) == 0 {
+		panic("no return value specified for WithField")
+	}
+
+	var r0 *logrus.Entry
+	if rf, ok := ret.Get(0).(func(string, interface{}) *logrus.Entry); ok {
+		r0 = rf(key, value)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*logrus.Entry)
+		}
+	}
+
+	return r0
+}
+
+// WithFields provides a mock function with given fields: fields
+func (_m *FieldLogger) WithFields(fields logrus.Fields) *logrus.Entry {
+	ret := _m.Called(fields)
+
+	if len(ret) == 0 {
+		panic("no return value specified for WithFields")
+	}
+
+	var r0 *logrus.Entry
+	if rf, ok := ret.Get(0).(func(logrus.Fields) *logrus.Entry); ok {
+		r0 = rf(fields)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*logrus.Entry)
+		}
+	}
+
+	return r0
+}
+
+// NewFieldLogger creates a new instance of FieldLogger. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func NewFieldLogger(t interface {
+	mock.TestingT
+	Cleanup(func())
+},
+) *FieldLogger {
+	mock := &FieldLogger{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}