internal/task: add a step to run test for vscode-go extension

Add new method to interface CloudBuildClient: - RunCustomSteps accept a function as input parameter. - The input function should return a slice of cloud steps which will be eventually executed by the method. - The input function should accept an input parameter "resultURL" and can assume this will be valid so the caller can write output to it. - The method will generate a random result url and pass that to the input function to generate the cloud steps. A local relui screenshot is at https://github.com/golang/vscode-go/issues/3500#issuecomment-2341760224 For golang/vscode-go#3500 Change-Id: Icdc6411ab8274b71783c15a0ccf7ecae0d61b3a5 Reviewed-on: https://go-review.googlesource.com/c/build/+/610375 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2024-09-03 19:20:14 +00:00 · 2024-09-03 19:20:14 +00:00 · e0ba88339e
--- a/cmd/relui/main.go
+++ b/cmd/relui/main.go
@ -313,6 +313,7 @@ func main() {
 			V4: githubv4.NewClient(githubHTTPClient),
 		},
 		Gerrit:        gerritClient,
+		CloudBuild:    cloudBuildClient,
 		ApproveAction: relui.ApproveActionDep(dbPool),
 	}
 	dh.RegisterDefinition("Create a vscode-go release candidate", releaseVSCodeGoTasks.NewPrereleaseDefinition())
--- a/internal/task/cloudbuild.go
+++ b/internal/task/cloudbuild.go
@ -28,6 +28,13 @@ type CloudBuildClient interface {
 	// If gerritProject is provided, the script operates within a checkout of the
 	// latest commit on the default branch of that repository.
 	RunScript(ctx context.Context, script string, gerritProject string, outputs []string) (CloudBuild, error)
+	// RunCustomSteps is a low-level API that provides direct control over
+	// individual Cloud Build steps. It creates a random result directory
+	// and provides that as a parameter to the steps function, so that it
+	// may write output to it with 'gsutil cp' for accessing via ResultFS.
+	// Prefer RunScript for simpler scenarios.
+	// Reference: https://cloud.google.com/build/docs/build-config-file-schema
+	RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep) (CloudBuild, error)
 	// Completed reports whether a build has finished, returning an error if
 	// it's failed. It's suitable for use with AwaitCondition.
 	Completed(ctx context.Context, build CloudBuild) (detail string, completed bool, _ error)
@ -78,35 +85,32 @@ archive=$(wget -qO - 'https://go.dev/dl/?mode=json' | grep -Eo 'go.*linux-amd64.
 wget -qO - https://go.dev/dl/${archive} | tar -xz
 mv go /workspace/released_go
 `
-
 	const scriptPrefix = `#!/usr/bin/env bash
 set -eux
 set -o pipefail
 export PATH=/workspace/released_go/bin:$PATH
 `

-	// Cloud build loses directory structure when it saves artifacts, which is
-	// a problem since (e.g.) we have multiple files named go.mod in the
-	// tagging tasks. It's not very complicated, so reimplement it ourselves.
-	resultURL := fmt.Sprintf("%v/script-build-%v", c.ScratchURL, rand.Int63())
-	var saveOutputsScript strings.Builder
-	saveOutputsScript.WriteString(scriptPrefix)
-	for _, out := range outputs {
-		saveOutputsScript.WriteString(fmt.Sprintf("gsutil cp %q %q\n", out, resultURL+"/"+strings.TrimPrefix(out, "./")))
-	}
+	steps := func(resultURL string) []*cloudbuildpb.BuildStep {
+		// Cloud build loses directory structure when it saves artifacts, which is
+		// a problem since (e.g.) we have multiple files named go.mod in the
+		// tagging tasks. It's not very complicated, so reimplement it ourselves.
+		var saveOutputsScript strings.Builder
+		saveOutputsScript.WriteString(scriptPrefix)
+		for _, out := range outputs {
+			saveOutputsScript.WriteString(fmt.Sprintf("gsutil cp %q %q\n", out, resultURL+"/"+strings.TrimPrefix(out, "./")))
+		}

-	var steps []*cloudbuildpb.BuildStep
-	var dir string
-	if gerritProject != "" {
-		steps = append(steps, &cloudbuildpb.BuildStep{
-			Name: "gcr.io/cloud-builders/git",
-			Args: []string{"clone", "https://go.googlesource.com/" + gerritProject, "checkout"},
-		})
-		dir = "checkout"
-	}
-
-	build := &cloudbuildpb.Build{
-		Steps: append(steps,
+		var steps []*cloudbuildpb.BuildStep
+		var dir string
+		if gerritProject != "" {
+			steps = append(steps, &cloudbuildpb.BuildStep{
+				Name: "gcr.io/cloud-builders/git",
+				Args: []string{"clone", "https://go.googlesource.com/" + gerritProject, "checkout"},
+			})
+			dir = "checkout"
+		}
+		steps = append(steps,
 			&cloudbuildpb.BuildStep{
 				Name:   "bash",
 				Script: downloadGoScript,
@ -121,7 +125,17 @@ export PATH=/workspace/released_go/bin:$PATH
 				Script: saveOutputsScript.String(),
 				Dir:    dir,
 			},
-		),
+		)
+
+		return steps
+	}
+
+	return c.RunCustomSteps(ctx, steps)
+}
+
+func (c *RealCloudBuildClient) RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep) (CloudBuild, error) {
+	build := &cloudbuildpb.Build{
+		Steps: steps(fmt.Sprintf("%v/script-build-%v", c.ScratchURL, rand.Int63())),
 		Options: &cloudbuildpb.BuildOptions{
 			MachineType: cloudbuildpb.BuildOptions_E2_HIGHCPU_8,
 			Logging:     cloudbuildpb.BuildOptions_CLOUD_LOGGING_ONLY,
@ -142,8 +156,7 @@ export PATH=/workspace/released_go/bin:$PATH
 	if err != nil {
 		return CloudBuild{}, fmt.Errorf("reading metadata: %w", err)
 	}
-	return CloudBuild{Project: c.ScriptProject, ID: meta.Build.Id, ResultURL: resultURL}, nil
-
+	return CloudBuild{Project: c.ScriptProject, ID: meta.Build.Id}, nil
 }

 func (c *RealCloudBuildClient) Completed(ctx context.Context, build CloudBuild) (string, bool, error) {
--- a/internal/task/fakes.go
+++ b/internal/task/fakes.go
@ -30,6 +30,7 @@ import (
 	"testing"
 	"time"

+	"cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb"
 	"github.com/google/go-github/v48/github"
 	"github.com/google/uuid"
 	"github.com/shurcooL/githubv4"
@ -631,6 +632,12 @@ case "$1" in
 esac
 `

+const fakeChown = `
+#!/bin/bash -eux
+echo "chown change owner successful"
+exit 0
+`
+
 func NewFakeCloudBuild(t *testing.T, gerrit *FakeGerrit, project string, allowedTriggers map[string]map[string]string, fakeGo string) *FakeCloudBuild {
 	toolDir := t.TempDir()
 	if err := os.WriteFile(filepath.Join(toolDir, "go"), []byte(fakeGo), 0777); err != nil {
@ -639,6 +646,9 @@ func NewFakeCloudBuild(t *testing.T, gerrit *FakeGerrit, project string, allowed
 	if err := os.WriteFile(filepath.Join(toolDir, "gsutil"), []byte(fakeGsutil), 0777); err != nil {
 		t.Fatal(err)
 	}
+	if err := os.WriteFile(filepath.Join(toolDir, "chown"), []byte(fakeChown), 0777); err != nil {
+		t.Fatal(err)
+	}
 	return &FakeCloudBuild{
 		t:               t,
 		gerrit:          gerrit,
@ -742,6 +752,44 @@ func (cb *FakeCloudBuild) RunScript(ctx context.Context, script string, gerritPr
 	return CloudBuild{Project: cb.project, ID: id, ResultURL: "file://" + resultDir}, nil
 }

+func (cb *FakeCloudBuild) RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep) (CloudBuild, error) {
+	var gerritProject, fullScript string
+	resultURL := "file://" + cb.t.TempDir()
+	for _, step := range steps(resultURL) {
+		tool, found := strings.CutPrefix(step.Name, "gcr.io/cloud-builders/")
+		if !found {
+			return CloudBuild{}, fmt.Errorf("does not support custom image: %s", step.Name)
+		}
+		if tool == "git" && len(step.Args) > 0 && step.Args[0] == "clone" {
+			for _, arg := range step.Args {
+				project, found := strings.CutPrefix(arg, "https://go.googlesource.com/")
+				if found {
+					gerritProject = project
+					break
+				}
+			}
+			continue
+		}
+		if len(step.Args) > 0 {
+			fullScript += tool + " " + strings.Join(step.Args, " ") + "\n"
+		}
+		if step.Script != "" {
+			fullScript += step.Script + "\n"
+		}
+	}
+	// In real CloudBuild client, the RunScript calls this lower level method.
+	build, err := cb.RunScript(ctx, fullScript, gerritProject, nil)
+	if err != nil {
+		return CloudBuild{}, err
+	}
+	// Overwrites the ResultURL as the actual output is written to a unique result
+	// directory generated by this method.
+	// Unit tests should verify the contents of this directory.
+	// The ResultURL returned by RunScript is not used for output and will always
+	// point to a new, empty directory.
+	return CloudBuild{ID: build.ID, Project: build.Project, ResultURL: resultURL}, nil
+}
+
 type FakeSwarmingClient struct {
 	t       *testing.T
 	toolDir string
--- a/internal/task/releasegopls.go
+++ b/internal/task/releasegopls.go
@ -593,9 +593,9 @@ func (r *ReleaseGoplsTasks) possibleGoplsVersions(ctx *wf.TaskContext) ([]string
 	var possible []string
 	for _, v := range releaseVersions {
 		for _, next := range []releaseVersion{
-			{v.Major+1, 0, 0},             // next major
-			{v.Major, v.Minor+1, 0},       // next minor
-			{v.Major, v.Minor, v.Patch+1}, // next patch
+			{v.Major + 1, 0, 0},             // next major
+			{v.Major, v.Minor + 1, 0},       // next minor
+			{v.Major, v.Minor, v.Patch + 1}, // next patch
 		} {
 			if _, ok := seen[next]; !ok {
 				possible = append(possible, next.String())
@ -673,7 +673,7 @@ func (r *ReleaseGoplsTasks) updateVSCodeGoGoplsVersion(ctx *wf.TaskContext, revi
 	}
 	branches := []string{"master"}
 	if prerelease == "" {
-		releaseBranch, err := vsCodeGoActiveReleaseBranch(ctx, r.Gerrit)
+		releaseBranch, err := vscodeGoActiveReleaseBranch(ctx, r.Gerrit)
 		if err != nil {
 			return nil, err
 		}
--- a/internal/task/releasevscodego.go
+++ b/internal/task/releasevscodego.go
@ -12,6 +12,7 @@ import (
 	"strconv"
 	"strings"

+	"cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb"
 	"github.com/google/go-github/v48/github"
 	"golang.org/x/build/gerrit"
 	"golang.org/x/build/internal/relui/groups"
@ -52,6 +53,7 @@ import (
 type ReleaseVSCodeGoTasks struct {
 	Gerrit        GerritClient
 	GitHub        GitHubClientInterface
+	CloudBuild    CloudBuildClient
 	ApproveAction func(*wf.TaskContext) error
 }

@ -68,11 +70,11 @@ var nextVersionParam = wf.ParamDef[string]{
 }

 //go:embed template/vscode-go-release-issue.md
-var vscodeGOReleaseIssueTmplStr string
+var vscodeGoReleaseIssueTmplStr string

-// vsCodeGoActiveReleaseBranch returns the current active release branch in
+// vscodeGoActiveReleaseBranch returns the current active release branch in
 // vscode-go project.
-func vsCodeGoActiveReleaseBranch(ctx *wf.TaskContext, gerrit GerritClient) (string, error) {
+func vscodeGoActiveReleaseBranch(ctx *wf.TaskContext, gerrit GerritClient) (string, error) {
 	branches, err := gerrit.ListBranches(ctx, "vscode-go")
 	if err != nil {
 		return "", err
@ -132,17 +134,78 @@ func (r *ReleaseVSCodeGoTasks) NewPrereleaseDefinition() *wf.Definition {

 	release := wf.Task1(wd, "determine the release version", r.determineReleaseVersion, versionBumpStrategy)
 	prerelease := wf.Task1(wd, "find the next pre-release version", r.nextPrereleaseVersion, release)
+	revision := wf.Task2(wd, "find the revision for the pre-release version", r.findRevision, release, prerelease)
 	approved := wf.Action2(wd, "await release coordinator's approval", r.approvePrereleaseVersion, release, prerelease)

-	_ = wf.Task1(wd, "create release milestone and issue", r.createReleaseMilestoneAndIssue, release, wf.After(approved))
+	verified := wf.Action1(wd, "verify the release candidate", r.verifyTestResults, revision, wf.After(approved))

-	branched := wf.Action2(wd, "create release branch", r.createReleaseBranch, release, prerelease, wf.After(approved))
-	// TODO(hxjiang): replace empty commit with the branch's head once verified.
-	_ = wf.Action3(wd, "tag release candidate", r.tag, wf.Const(""), release, prerelease, wf.After(branched))
+	_ = wf.Task1(wd, "create release milestone and issue", r.createReleaseMilestoneAndIssue, release, wf.After(verified))
+	branched := wf.Action2(wd, "create release branch", r.createReleaseBranch, release, prerelease, wf.After(verified))
+	_ = wf.Action3(wd, "tag release candidate", r.tag, revision, release, prerelease, wf.After(branched))

 	return wd
 }

+// findRevision determines the appropriate revision for the current release.
+// Returns the head of the master branch if this is the first release candidate
+// for a stable minor version (as no release branch exists yet).
+// Returns the head of the corresponding release branch otherwise.
+func (r *ReleaseVSCodeGoTasks) findRevision(ctx *wf.TaskContext, release releaseVersion, prerelease string) (string, error) {
+	branch := fmt.Sprintf("release-v%v.%v", release.Major, release.Minor)
+	if release.Patch == 0 && prerelease == "rc.1" {
+		branch = "master"
+	}
+
+	return r.Gerrit.ReadBranchHead(ctx, "vscode-go", branch)
+}
+
+func (r *ReleaseVSCodeGoTasks) verifyTestResults(ctx *wf.TaskContext, revision string) error {
+	// We are running all tests in a docker as a user 'node' (uid: 1000)
+	// Let the user own the directory.
+	testScript := `#!/usr/bin/env bash
+set -eux
+set -o pipefail
+
+chown -R 1000:1000 .
+./build/all.bash testlocal &> output.log
+`
+
+	build, err := r.CloudBuild.RunCustomSteps(ctx, func(resultURL string) []*cloudbuildpb.BuildStep {
+		return []*cloudbuildpb.BuildStep{
+			{
+				Name: "gcr.io/cloud-builders/git",
+				Args: []string{"clone", "https://go.googlesource.com/vscode-go", "vscode-go"},
+			},
+			{
+				Name: "gcr.io/cloud-builders/git",
+				Args: []string{"checkout", revision},
+				Dir:  "vscode-go",
+			},
+			{
+				Name:   "gcr.io/cloud-builders/docker",
+				Script: testScript,
+				Dir:    "vscode-go",
+			},
+			{
+				Name: "gcr.io/cloud-builders/gsutil",
+				Args: []string{"cp", "output.log", fmt.Sprintf("%s/output.log", resultURL)},
+				Dir:  "vscode-go",
+			},
+		}
+	})
+	if err != nil {
+		return err
+	}
+
+	outputs, err := buildToOutputs(ctx, r.CloudBuild, build)
+	if err != nil {
+		return err
+	}
+
+	ctx.Printf("the output from test run:\n%s\n", outputs["output.log"])
+	return nil
+}
+
 func (r *ReleaseVSCodeGoTasks) createReleaseMilestoneAndIssue(ctx *wf.TaskContext, semv releaseVersion) (int, error) {
 	version := fmt.Sprintf("v%v.%v.%v", semv.Major, semv.Minor, semv.Patch)

@ -168,7 +231,7 @@ func (r *ReleaseVSCodeGoTasks) createReleaseMilestoneAndIssue(ctx *wf.TaskContex
 		}
 	}

-	content := fmt.Sprintf(vscodeGOReleaseIssueTmplStr, version)
+	content := fmt.Sprintf(vscodeGoReleaseIssueTmplStr, version)
 	issue, _, err := r.GitHub.CreateIssue(ctx, "golang", "vscode-go", &github.IssueRequest{
 		Title:     &title,
 		Body:      &content,
--- a/internal/task/releasevscodego_test.go
+++ b/internal/task/releasevscodego_test.go
@ -346,13 +346,80 @@ func TestVSCodeGoActiveReleaseBranch(t *testing.T) {
 				Context: context.Background(),
 				Logger:  &testLogger{t, ""},
 			}
-			got, err := vsCodeGoActiveReleaseBranch(ctx, gerrit)
+			got, err := vscodeGoActiveReleaseBranch(ctx, gerrit)
 			if err != nil {
 				t.Fatal(err)
 			}

 			if tc.want != got {
-				t.Errorf("vsCodeGoActiveReleaseBranch() = %q, want %q", got, tc.want)
+				t.Errorf("vscodeGoActiveReleaseBranch() = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestVerifyTestResults(t *testing.T) {
+	mustHaveShell(t)
+	fakeScriptFmt := `#!/bin/bash -exu
+
+case "$1" in
+"testlocal")
+	echo "the testlocal return %v"
+	exit %v
+	;;
+*)
+	echo unexpected command $@
+	exit 1
+	;;
+esac
+`
+	testcases := []struct {
+		name    string
+		rc      int
+		wantErr bool
+	}{
+		{
+			name:    "test failed, return error",
+			rc:      1,
+			wantErr: true,
+		},
+		{
+			name:    "test passed, return nil",
+			rc:      0,
+			wantErr: false,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			vscodego := NewFakeRepo(t, "vscode-go")
+			commit := vscodego.Commit(map[string]string{
+				"go.mod":         "module github.com/golang/vscode-go\n",
+				"go.sum":         "\n",
+				"build/all.bash": fmt.Sprintf(fakeScriptFmt, tc.rc, tc.rc),
+			})
+			// Overwrite the script to empty to make sure vscode-go flow will checkout
+			// the specific commit.
+			_ = vscodego.Commit(map[string]string{
+				"build/all.bash": "",
+			})
+
+			gerrit := NewFakeGerrit(t, vscodego)
+			ctx := &workflow.TaskContext{
+				Context: context.Background(),
+				Logger:  &testLogger{t, ""},
+			}
+
+			tasks := &ReleaseVSCodeGoTasks{
+				Gerrit:     gerrit,
+				CloudBuild: NewFakeCloudBuild(t, gerrit, "vscode-go", nil, ""),
+			}
+
+			err := tasks.verifyTestResults(ctx, commit)
+			if tc.wantErr && err == nil {
+				t.Errorf("verifyTestResult() should return error but return nil")
+			} else if !tc.wantErr && err != nil {
+				t.Errorf("verifyTestResult() should return nil but return err: %v", err)
 			}
 		})
 	}