Sep 24, 2024 · Sep 17, 2024 · Sep 17, 2024 · Sep 17, 2024 · Sep 18, 2024 · Sep 18, 2024
diff --git a/agent/agentscripts/agentscripts_test.go b/agent/agentscripts/agentscripts_test.go
 require.ErrorIs(t, runner.Execute(context.Background(), nil), agentscripts.ErrTimeout)
 }

 func TestScriptReportsTiming(t *testing.T) {
 t.Parallel()

 tests := []struct {
 displayName string
 script      string
 exitCode    int32
 }{
 {
 displayName: "exit-0",
 script:      "exit 0",
 exitCode:    0,
 },
 {
 displayName: "exit-1",
 script:      "exit 1",
 exitCode:    1,
 },
 }

 for _, tt := range tests {
 ctx := testutil.Context(t, testutil.WaitShort)
 fLogger := newFakeScriptLogger()
 runner := setup(t, func(uuid2 uuid.UUID) agentscripts.ScriptLogger {
 return fLogger
 })
 defer runner.Close()
 aAPI := agenttest.NewFakeAgentAPI(t, slogtest.Make(t, nil), nil, nil)
 err := runner.Init([]codersdk.WorkspaceAgentScript{{
 DisplayName: tt.displayName,
 LogSourceID: uuid.New(),
 Script:      tt.script,
 }}, aAPI.ScriptCompleted)
 require.NoError(t, err)
 require.NoError(t, runner.Execute(context.Background(), func(script codersdk.WorkspaceAgentScript) bool {
 return true
 }))
 _ = testutil.RequireRecvCtx(ctx, t, fLogger.logs)

 require.Equal(t, len(aAPI.GetTiming()), 1)
 timing := aAPI.GetTiming()[0]
 require.Equal(t, timing.DisplayName, tt.displayName)
 require.Equal(t, timing.ExitCode, tt.exitCode)
 }
 }

 // TestCronClose exists because cron.Run() can happen after cron.Close().
 // If this happens, there used to be a deadlock.
 func TestCronClose(t *testing.T) {
diff --git a/agent/agenttest/client.go b/agent/agenttest/client.go
 logsCh          chan<- *agentproto.BatchCreateLogsRequest
 lifecycleStates []codersdk.WorkspaceAgentLifecycle
 metadata        map[string]agentsdk.Metadata
 timing          []*agentproto.Timing

 getAnnouncementBannersFunc func() ([]codersdk.BannerConfig, error)
 }
 return &agentproto.ServiceBanner{}, nil
 }

 func (f *FakeAgentAPI) GetTiming() []*agentproto.Timing {
 return f.timing
 }

 func (f *FakeAgentAPI) SetAnnouncementBannersFunc(fn func() ([]codersdk.BannerConfig, error)) {
 f.Lock()
 defer f.Unlock()
 return &agentproto.BatchCreateLogsResponse{}, nil
 }

 func (*FakeAgentAPI) ScriptCompleted(_ context.Context, _ *agentproto.WorkspaceAgentScriptCompletedRequest) (*agentproto.WorkspaceAgentScriptCompletedResponse, error) {
 func (f *FakeAgentAPI) ScriptCompleted(_ context.Context, req *agentproto.WorkspaceAgentScriptCompletedRequest) (*agentproto.WorkspaceAgentScriptCompletedResponse, error) {
 f.Lock()
 f.timing = append(f.timing, req.Timing)
 f.Unlock()

 return &agentproto.WorkspaceAgentScriptCompletedResponse{}, nil
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -116,6 +116,52 @@ func TestTimeout(t *testing.T) {
		require.ErrorIs(t, runner.Execute(context.Background(), nil), agentscripts.ErrTimeout)
		}

		func TestScriptReportsTiming(t *testing.T) {
dannykopping marked this conversation as resolved. Show resolvedHide resolved Copy link Contributor dannykoppingSep 20, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I think we need to expand this a bit to cover the timeout and non-zero exit status use-cases. mafredri reacted with thumbs up emoji Copy link ContributorAuthor DanielleMaywoodSep 20, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Yeah, definitely a good idea. I had tried a non-zero exit code but it appeared to cause the test to fail so I'll investigate that again to make sure it wasn't me doing something wrong. Copy link Member johnstcnSep 20, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Potentially a follow-up PR?
		t.Parallel()

		tests := []struct {
		displayName string
		script string
		exitCode int32
		}{
		{
		displayName: "exit-0",
		script: "exit 0",
		exitCode: 0,
		},
		{
		displayName: "exit-1",
		script: "exit 1",
		exitCode: 1,
		},
		}

		for _, tt := range tests {
		ctx := testutil.Context(t, testutil.WaitShort)
		fLogger := newFakeScriptLogger()
		runner := setup(t, func(uuid2 uuid.UUID) agentscripts.ScriptLogger {
		return fLogger
		})
		defer runner.Close()
		aAPI := agenttest.NewFakeAgentAPI(t, slogtest.Make(t, nil), nil, nil)
		err := runner.Init([]codersdk.WorkspaceAgentScript{{
		DisplayName: tt.displayName,
		LogSourceID: uuid.New(),
		Script: tt.script,
		}}, aAPI.ScriptCompleted)
		require.NoError(t, err)
		require.NoError(t, runner.Execute(context.Background(), func(script codersdk.WorkspaceAgentScript) bool {
		return true
		}))
		_ = testutil.RequireRecvCtx(ctx, t, fLogger.logs)

		require.Equal(t, len(aAPI.GetTiming()), 1)
		timing := aAPI.GetTiming()[0]
		require.Equal(t, timing.DisplayName, tt.displayName)
		require.Equal(t, timing.ExitCode, tt.exitCode)
		}
		}

		// TestCronClose exists because cron.Run() can happen after cron.Close().
		// If this happens, there used to be a deadlock.
		func TestCronClose(t *testing.T) {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -170,6 +170,7 @@ type FakeAgentAPI struct {
		logsCh chan<- *agentproto.BatchCreateLogsRequest
		lifecycleStates []codersdk.WorkspaceAgentLifecycle
		metadata map[string]agentsdk.Metadata
		timing []*agentproto.Timing

		getAnnouncementBannersFunc func() ([]codersdk.BannerConfig, error)
		}
Expand All		@@ -182,6 +183,10 @@ func (FakeAgentAPI) GetServiceBanner(context.Context, agentproto.GetServiceBan
		return &agentproto.ServiceBanner{}, nil
		}

		func (f FakeAgentAPI) GetTiming() []agentproto.Timing {
		return f.timing
		}

		func (f *FakeAgentAPI) SetAnnouncementBannersFunc(fn func() ([]codersdk.BannerConfig, error)) {
		f.Lock()
		defer f.Unlock()
Expand DownExpand Up		@@ -301,7 +306,11 @@ func (f FakeAgentAPI) BatchCreateLogs(ctx context.Context, req agentproto.Batc
		return &agentproto.BatchCreateLogsResponse{}, nil
		}

		func (FakeAgentAPI) ScriptCompleted(_ context.Context, _ agentproto.WorkspaceAgentScriptCompletedRequest) (*agentproto.WorkspaceAgentScriptCompletedResponse, error) {
		func (f FakeAgentAPI) ScriptCompleted(_ context.Context, req agentproto.WorkspaceAgentScriptCompletedRequest) (*agentproto.WorkspaceAgentScriptCompletedResponse, error) {
		f.Lock()
		f.timing = append(f.timing, req.Timing)
		f.Unlock()

		return &agentproto.WorkspaceAgentScriptCompletedResponse{}, nil
		}

Expand Down