Add/improve unit tests for agentendpoint/exec_task.go by ganochenkodg · Pull Request #933 · GoogleCloudPlatform/osconfig

ganochenkodg · 2026-03-19T16:53:07Z

With this PR better test coverage for ./agentendpoint/exec_task.go was reached:

github.com/GoogleCloudPlatform/osconfig/agentendpoint/exec_task.go:66: getGCSObject 0.0% -> 92.9%
github.com/GoogleCloudPlatform/osconfig/agentendpoint/exec_task.go:89: executeCommand 63.6% -> 100.0%
github.com/GoogleCloudPlatform/osconfig/agentendpoint/exec_task.go:236: RunExecStep 0.0% -> 100.0%

Average test coverage for this file 51.88% -> 90.01%

ganochenkodg · 2026-03-19T16:53:30Z

This is draft PR, do not review it right now please.

google-oss-prow · 2026-03-19T16:53:31Z

Hi @ganochenkodg. Thanks for your PR.

I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

google-oss-prow · 2026-03-19T17:03:01Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ganochenkodg
Once this PR has been reviewed and has the lgtm label, please assign savija-tv for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

iliatsuprik

I've got 79% via go test -coverprofile=coverage.out ./agentendpoint && go tool cover -html=coverage.out. Could you check if some code branches can be covered

iliatsuprik · 2026-03-20T19:40:35Z

agentendpoint/exec_task_test.go

+		t.Run(tt.name, func(t *testing.T) {
+			run = tt.mockRun
+			gotCode, err := executeCommand(ctx, "test-path", nil)
+			if (err != nil) != tt.wantErr {


could you check exact error text when possible

iliatsuprik · 2026-03-20T19:41:15Z

agentendpoint/exec_task_test.go

+	}
+}
+
+func TestExecuteCommand(t *testing.T) {


could you add a docstring to the test with a short description what it's doing

Sure, added for both TestExecuteCommand and TestGCS funcs

iliatsuprik · 2026-03-20T20:00:09Z

agentendpoint/exec_task_test.go

+			},
+			wantCode: 0,
+			wantErr:  false,
+		},


here we validate errors in result, but do not validate a success testcase

Added Success case

iliatsuprik · 2026-03-20T20:04:54Z

agentendpoint/exec_task_test.go

+			defer ts.Close()
+
+			os.Setenv("STORAGE_EMULATOR_HOST", ts.URL)
+			defer os.Unsetenv("STORAGE_EMULATOR_HOST")


Unset env will remove STORAGE_EMULATOR_HOST value, it's better to restore the initial value which existed before the test started (to avoid flaky tests). I propose to use a function similar to https://github.com/GoogleCloudPlatform/osconfig/pull/925/changes#diff-783605524de2c2d44922219ca97cd7cbee5b401a62efdf763bc00083728780c6R1322-R1341

Good idea, use this function now

burov · 2026-03-23T13:34:57Z

/gcbrun

burov · 2026-03-25T13:42:05Z

/gcbrun

burov · 2026-03-25T13:48:11Z

agentendpoint/exec_task_test.go

+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte(testContent))
+			},
+			wantErr: true,


Is there anything blocking us from verifying an actual error instead of just it's presence ?

burov · 2026-03-25T13:48:59Z

agentendpoint/exec_task_test.go

+			ts := httptest.NewServer(tt.handler)
+			defer ts.Close()
+
+      rollback := OverrideEnv(t, "STORAGE_EMULATOR_HOST", ts.URL)


Fix format please.

gofmt? fixed now

burov · 2026-03-25T13:50:59Z

agentendpoint/exec_task_test.go

+			}
+			if err == nil {
+				defer os.Remove(localPath)
+				content, err := os.ReadFile(localPath)


I suppose util functions from https://github.com/GoogleCloudPlatform/osconfig/pull/934/changes#diff-b794709cfa990f89da0485b8f4526aa8c890463db3dce888ee66cfaccff93d18 should came handy

Yep, using them now

burov · 2026-03-25T13:51:51Z

agentendpoint/exec_task_test.go

+			if (err != nil) != tt.wantErr {
+				t.Errorf("%s: executeCommand() error = %v, wantErr %v", tt.name, err, tt.wantErr)
+			}
+			if err != nil && tt.wantErrMsg != "" && !strings.Contains(err.Error(), tt.wantErrMsg) {


Same about errors and result check https://github.com/GoogleCloudPlatform/osconfig/pull/934/changes#diff-b794709cfa990f89da0485b8f4526aa8c890463db3dce888ee66cfaccff93d18 should be handy

ganochenkodg marked this pull request as draft March 19, 2026 16:53

google-oss-prow bot added the size/L label Mar 19, 2026

google-oss-prow bot requested review from paulinakania and zoltak-g March 19, 2026 16:53

google-oss-prow bot added the needs-ok-to-test label Mar 19, 2026

ganochenkodg force-pushed the agentendpoint-exectask-unittests branch from 63267c8 to 3d8c42c Compare March 19, 2026 16:57

google-oss-prow bot added size/XXL size/XS size/L and removed size/L size/XXL size/XS labels Mar 19, 2026

add tests

a7a5ea6

ganochenkodg force-pushed the agentendpoint-exectask-unittests branch from 905b95c to a7a5ea6 Compare March 19, 2026 17:04

iliatsuprik reviewed Mar 20, 2026

View reviewed changes

Merge branch 'master' into agentendpoint-exectask-unittests

44a940d

resolve review comments

17527cb

burov reviewed Mar 25, 2026

View reviewed changes

ganochenkodg and others added 5 commits March 26, 2026 04:48

update header

6edc8a5

Merge branch 'master' into agentendpoint-exectask-unittests

e638636

use functions from utiltest.go

d378652

comments

9ef57de

format

bb28daf

ganochenkodg marked this pull request as ready for review March 27, 2026 01:45

google-oss-prow bot requested a review from savija-tv March 27, 2026 01:45

Conversation

ganochenkodg commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ganochenkodg commented Mar 19, 2026

Uh oh!

google-oss-prow bot commented Mar 19, 2026

Uh oh!

google-oss-prow bot commented Mar 19, 2026

Uh oh!

iliatsuprik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

burov commented Mar 23, 2026

Uh oh!

burov commented Mar 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ganochenkodg commented Mar 19, 2026 •

edited

Loading