test(logging): add logtest.NewLogger (#1300)

Return a `Logger` and its recording `Handler` together, so tests no longer need the two-line setup.

Stacked on https://gitea.com/gitea/runner/pulls/1295.

Assisted-by: Codet
Co-authored-by: bircni <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1300
Reviewed-by: bircni <bircni@icloud.com>

act/common/logging/logging_test.go

@@ -92,8 +92,7 @@ func frameOf(t *testing.T, pc uintptr) runtime.Frame {
 }
 
 func TestCallerPC(t *testing.T) {
-	h := logtest.New()
-	l := logging.New(h)
+	l, h := logtest.NewLogger()
 	l.Infof("method")
 	l.Info("method")
 	for _, r := range h.AllRecords() {

act/common/logging/logtest/logtest.go

@@ -29,6 +29,12 @@ func New() *Handler {
 	return NewWithLevel(logging.LevelInfo)
 }
 
+// NewLogger returns a Logger writing to a new Handler from New, and that Handler.
+func NewLogger() (*logging.Logger, *Handler) {
+	h := New()
+	return logging.New(h), h
+}
+
 func NewWithLevel(minLevel logging.Level) *Handler {
 	return &Handler{store: &store{}, minLevel: minLevel}
 }

act/container/docker_run_test.go

@@ -354,8 +354,7 @@ func TestRemoveIgnoresAutoRemoveRace(t *testing.T) {
 		{name: "real failure", err: removeFailure, wantErr: removeFailure},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			hook := logtest.New()
-			logger := logging.New(hook)
+			logger, hook := logtest.NewLogger()
 			ctx := common.WithLogger(context.Background(), logger)
 			client := &mockDockerClient{}
 			client.On("ContainerKill", ctx, "abc", killOpts).Return(mobyclient.ContainerKillResult{}, nil)
@@ -616,8 +615,7 @@ func TestCheckVolumes(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			hook := logtest.New()
-			logger := logging.New(hook)
+			logger, hook := logtest.NewLogger()
 			ctx := common.WithLogger(context.Background(), logger)
 			cr := &containerReference{
 				input: &NewContainerInput{
@@ -716,8 +714,7 @@ func TestSanitizeOptionsHostConfig(t *testing.T) {
 // mergeOptions merges both option sources into a bare container, returning the result and its log.
 func mergeOptions(t *testing.T, runnerOptions, workflowOptions string, privileged bool) (*container.HostConfig, *logtest.Handler) {
 	t.Helper()
-	hook := logtest.New()
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 	cr := &containerReference{input: &NewContainerInput{
 		RunnerOptions:   runnerOptions,
 		WorkflowOptions: workflowOptions,
@@ -810,8 +807,7 @@ func TestSanitizeConfigKeepsTmpfsMounts(t *testing.T) {
 	require.Len(t, hostConfig.Mounts, 1)
 
 	for _, validVolumes := range [][]string{nil, {"data"}, {"**:ro"}} {
-		hook := logtest.New()
-		logger := logging.New(hook)
+		logger, hook := logtest.NewLogger()
 		_, sanitized := (&containerReference{input: &NewContainerInput{ValidVolumes: validVolumes}}).sanitizeConfig(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{Mounts: []mount.Mount{
 			hostConfig.Mounts[0],
 			{Type: mount.TypeVolume, Target: "/anonymous"},
@@ -936,8 +932,7 @@ func TestMergeContainerConfigsWarnsOnlyAboutOptionsThatWereGiven(t *testing.T) {
 func TestDockerCreateKeepsInputEntrypoint(t *testing.T) {
 	for _, entrypoint := range [][]string{nil, {"sleep", "3600"}} {
 		for _, optionEntrypoint := range []string{"/sbin/dumb-init", ""} {
-			hook := logtest.New()
-			logger := logging.New(hook)
+			logger, hook := logtest.NewLogger()
 			cr := &containerReference{
 				input: &NewContainerInput{
 					NetworkMode:     "bridge",

act/runner/command_test.go

@@ -12,7 +12,6 @@ import (
 	"testing"
 
 	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/common/logging"
 	"gitea.com/gitea/runner/act/common/logging/logtest"
 
 	"gitea.dev/actionslib/pkg/model"
@@ -27,8 +26,7 @@ func unsecureRC() *RunContext {
 
 func TestSetEnv(t *testing.T) {
 	a := assert.New(t)
-	hook := logtest.New()
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 	ctx := common.WithLogger(context.Background(), logger)
 	rc := unsecureRC()
 	handler := rc.commandHandler(ctx)
@@ -122,8 +120,7 @@ func TestAddpath(t *testing.T) {
 }
 
 func TestStopCommands(t *testing.T) {
-	hook := logtest.New()
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 
 	a := assert.New(t)
 	ctx := common.WithLogger(context.Background(), logger)
@@ -177,8 +174,7 @@ func TestAddpathADO(t *testing.T) {
 }
 
 func TestAddmask(t *testing.T) {
-	hook := logtest.New()
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 
 	a := assert.New(t)
 	ctx := context.Background()

act/runner/job_executor_test.go

@@ -262,9 +262,7 @@ func TestNewJobExecutorDownloadsAllActionsBeforeTheFirstStep(t *testing.T) {
 		defer sm.AssertExpectations(t)
 	}
 
-	hook := logtest.New()
-
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 	err := newJobExecutor(jim, sfm, rc)(common.WithLogger(ctx, logger.With("job", "test")))
 	require.NoError(t, err)
 
@@ -969,8 +967,7 @@ func TestReadSingleFileFromContainerArchiveFindsMatchingRegularFile(t *testing.T
 }
 
 func TestReadSingleFileFromContainerArchiveTruncatesWhenTooLarge(t *testing.T) {
-	hook := logtest.New()
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 	ctx := common.WithLogger(context.Background(), logger)
 	cm := &containerMock{}
 	content := strings.Repeat("a", 300)
@@ -995,8 +992,7 @@ func TestReadSingleFileFromContainerArchiveTruncatesWhenTooLarge(t *testing.T) {
 }
 
 func TestReadSingleFileFromContainerArchiveKeepsExactLimitWithoutWarning(t *testing.T) {
-	hook := logtest.New()
-	logger := logging.New(hook)
+	logger, hook := logtest.NewLogger()
 	ctx := common.WithLogger(context.Background(), logger)
 	cm := &containerMock{}
 	cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/SUMMARY.md").Return(

act/runner/run_context_test.go

@@ -1399,8 +1399,7 @@ func TestRunContext_cleanupFailedStart(t *testing.T) {
 				Run:           &model.Run{JobID: "job", Workflow: workflow},
 				platformImage: "node:20",
 			}
-			hook := logtest.New()
-			logger := logging.New(hook)
+			logger, hook := logtest.NewLogger()
 			ctx := common.WithLogger(common.WithDryrun(t.Context(), true), logger)
 			rc.ExprEval = rc.NewExpressionEvaluator(ctx)
 			err = rc.startContainer().Then(rc.stopContainer()).Finally(rc.closeContainer())(ctx)

act/runner/step_test.go

@@ -513,8 +513,7 @@ func TestRunStepExecutorParity(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			step := newStep(t, test.step)
-			hook := logtest.New()
-			logger := logging.New(hook)
+			logger, hook := logtest.NewLogger()
 			err := runStepExecutor(step, stepStageMain, test.executor)(common.WithLogger(context.Background(), logger))
 
 			require.ErrorContains(t, err, test.wantError)
@@ -525,8 +524,7 @@ func TestRunStepExecutorParity(t *testing.T) {
 
 	t.Run("file command error honors continue-on-error", func(t *testing.T) {
 		step := newStep(t, &model.Step{ID: "commands", RawContinueOnError: "true"})
-		hook := logtest.New()
-		logger := logging.New(hook)
+		logger, hook := logtest.NewLogger()
 		err := runStepExecutor(step, stepStageMain, func(context.Context) error {
 			require.NoError(t, os.WriteFile(step.env["GITHUB_ENV"], []byte("GOOD=1\nmalformed\n"), 0o600))
 			require.NoError(t, os.WriteFile(step.env["GITHUB_OUTPUT"], []byte("kept=value\n"), 0o600))