internal/task: add new selection input to gopls pre-release flow

Coordinator have three options to run the pre-release flow:
* next minor or next patch, the flow will interpret the user intention.
* use explicit version, the user must input the version explicitly.

A local relui screenshot is at https://go.dev/issue/57643#issuecomment-2310730081

For golang/go#57643

Change-Id: I9621da87f248c79597641162682fb2d108bc19f9
Reviewed-on: https://go-review.googlesource.com/c/build/+/608415
Auto-Submit: Hongxiang Jiang <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: h9jiang

internal/task/releasegopls.go

@@ -33,22 +33,27 @@ type ReleaseGoplsTasks struct {
 func (r *ReleaseGoplsTasks) NewPrereleaseDefinition() *wf.Definition {
 	wd := wf.New(wf.ACL{Groups: []string{groups.ToolsTeam}})
 
-	// TODO(hxjiang): provide potential release versions in the relui where the
-	// coordinator can choose which version to release instead of manual input.
-	version := wf.Param(wd, wf.ParamDef[string]{Name: "version"})
+	// versionBumpStrategy specifies the desired release type: next minor or next
+	// patch.
+	// This should be the default choice for most releases.
+	versionBumpStrategy := wf.Param(wd, nextVersionParam)
+	// inputVersion allows manual override of the version, bypassing the version
+	// bump strategy.
+	// Use with caution.
+	inputVersion := wf.Param(wd, wf.ParamDef[string]{Name: "explicit version (optional)"})
 	reviewers := wf.Param(wd, reviewersParam)
 
-	semv := wf.Task1(wd, "validating input version", r.isValidReleaseVersion, version)
-	prerelease := wf.Task1(wd, "find the pre-release version", r.nextPrerelease, semv)
+	semv := wf.Task2(wd, "determine the version", r.determineVersion, inputVersion, versionBumpStrategy)
+	prerelease := wf.Task1(wd, "find the pre-release version", r.nextPrereleaseVersion, semv)
 	approved := wf.Action2(wd, "wait for release coordinator approval", r.approveVersion, semv, prerelease)
 
 	issue := wf.Task1(wd, "create release git issue", r.createReleaseIssue, semv, wf.After(approved))
-	branchCreated := wf.Action1(wd, "creating new branch if minor release", r.createBranchIfMinor, semv, wf.After(issue))
+	branchCreated := wf.Action1(wd, "create new branch if minor release", r.createBranchIfMinor, semv, wf.After(issue))
 
-	configChangeID := wf.Task3(wd, "updating branch's codereview.cfg", r.updateCodeReviewConfig, semv, reviewers, issue, wf.After(branchCreated))
+	configChangeID := wf.Task3(wd, "update branch's codereview.cfg", r.updateCodeReviewConfig, semv, reviewers, issue, wf.After(branchCreated))
 	configCommit := wf.Task1(wd, "await config CL submission", clAwaiter{r.Gerrit}.awaitSubmission, configChangeID)
 
-	dependencyChangeID := wf.Task4(wd, "updating gopls' x/tools dependency", r.updateXToolsDependency, semv, prerelease, reviewers, issue, wf.After(configCommit))
+	dependencyChangeID := wf.Task4(wd, "update gopls' x/tools dependency", r.updateXToolsDependency, semv, prerelease, reviewers, issue, wf.After(configCommit))
 	dependencyCommit := wf.Task1(wd, "await gopls' x/tools dependency CL submission", clAwaiter{r.Gerrit}.awaitSubmission, dependencyChangeID)
 
 	verified := wf.Action1(wd, "verify installing latest gopls using release branch dependency commit", r.verifyGoplsInstallation, dependencyCommit)
@@ -61,6 +66,58 @@ func (r *ReleaseGoplsTasks) NewPrereleaseDefinition() *wf.Definition {
 	return wd
 }
 
+// determineVersion returns the release version based on coordinator inputs.
+//
+// Returns the specified input version if provided; otherwise, interpret a new
+// version based on the version bumping strategy.
+func (r *ReleaseGoplsTasks) determineVersion(ctx *wf.TaskContext, inputVersion, versionBumpStrategy string) (semversion, error) {
+	switch versionBumpStrategy {
+	case "use explicit version":
+		if inputVersion == "" {
+			return semversion{}, fmt.Errorf("the input version should not be empty when choosing explicit version release")
+		}
+		if err := r.isValidReleaseVersion(ctx, inputVersion); err != nil {
+			return semversion{}, err
+		}
+		semv, ok := parseSemver(inputVersion)
+		if !ok {
+			return semversion{}, fmt.Errorf("input version %q can not be parsed as semantic version", inputVersion)
+		}
+		return semv, nil
+	case "next minor", "next patch":
+		return r.interpretNextRelease(ctx, versionBumpStrategy)
+	default:
+		return semversion{}, fmt.Errorf("unknown version selection strategy: %q", versionBumpStrategy)
+	}
+}
+
+func (r *ReleaseGoplsTasks) interpretNextRelease(ctx *wf.TaskContext, versionBumpStrategy string) (semversion, error) {
+	tags, err := r.Gerrit.ListTags(ctx, "tools")
+	if err != nil {
+		return semversion{}, err
+	}
+
+	var versions []string
+	for _, tag := range tags {
+		if v, ok := strings.CutPrefix(tag, "gopls/"); ok {
+			versions = append(versions, v)
+		}
+	}
+
+	version := latestVersion(versions, isReleaseVersion)
+	switch versionBumpStrategy {
+	case "next minor":
+		version.Minor += 1
+		version.Patch = 0
+	case "next patch":
+		version.Patch += 1
+	default:
+		return semversion{}, fmt.Errorf("unknown version selection strategy: %q", versionBumpStrategy)
+	}
+
+	return version, nil
+}
+
 func (r *ReleaseGoplsTasks) approveVersion(ctx *wf.TaskContext, semv semversion, pre string) error {
 	ctx.Printf("The next release candidate will be v%v.%v.%v-%s", semv.Major, semv.Minor, semv.Patch, pre)
 	return r.ApproveAction(ctx)
@@ -230,9 +287,9 @@ parent-branch: master
 	return r.Gerrit.CreateAutoSubmitChange(ctx, changeInput, reviewers, files)
 }
 
-// nextPrerelease inspects the tags in tools repo that match with the given
-// version and find the next prerelease version.
-func (r *ReleaseGoplsTasks) nextPrerelease(ctx *wf.TaskContext, semv semversion) (string, error) {
+// nextPrereleaseVersion inspects the tags in tools repo that match with the given
+// version and finds the next prerelease version.
+func (r *ReleaseGoplsTasks) nextPrereleaseVersion(ctx *wf.TaskContext, semv semversion) (string, error) {
 	cur, err := currentGoplsPrerelease(ctx, r.Gerrit, semv)
 	if err != nil {
 		return "", err
@@ -410,22 +467,21 @@ func (r *ReleaseGoplsTasks) mailAnnouncement(ctx *wf.TaskContext, semv semversio
 	return r.SendMail(r.AnnounceMailHeader, content)
 }
 
-func (r *ReleaseGoplsTasks) isValidReleaseVersion(ctx *wf.TaskContext, ver string) (semversion, error) {
+func (r *ReleaseGoplsTasks) isValidReleaseVersion(ctx *wf.TaskContext, ver string) error {
 	if !semver.IsValid(ver) {
-		return semversion{}, fmt.Errorf("the input %q version does not follow semantic version schema", ver)
+		return fmt.Errorf("the input %q version does not follow semantic version schema", ver)
 	}
 
 	versions, err := r.possibleGoplsVersions(ctx)
 	if err != nil {
-		return semversion{}, fmt.Errorf("failed to get latest Gopls version tags from x/tool: %w", err)
+		return fmt.Errorf("failed to get latest Gopls version tags from x/tool: %w", err)
 	}
 
 	if !slices.Contains(versions, ver) {
-		return semversion{}, fmt.Errorf("the input %q is not next version of any existing versions", ver)
+		return fmt.Errorf("the input %q is not next version of any existing versions", ver)
 	}
 
-	semver, _ := parseSemver(ver)
-	return semver, nil
+	return nil
 }
 
 // semversion is a parsed semantic version.

internal/task/releasegopls_test.go

@@ -17,6 +17,62 @@ import (
 	"golang.org/x/build/internal/workflow"
 )
 
+func TestInterpretNextRelease(t *testing.T) {
+	tests := []struct {
+		name string
+		tags []string
+		bump string
+		want semversion
+	}{
+		{
+			name: "next minor version of v0.0.0 is v0.1.0",
+			tags: []string{"gopls/v0.0.0"},
+			bump: "next minor",
+			want: semversion{Major: 0, Minor: 1, Patch: 0},
+		},
+		{
+			name: "pre-release versions should be ignored",
+			tags: []string{"gopls/v0.0.0", "gopls/v0.1.0-pre.1", "gopls/v0.1.0-pre.2"},
+			bump: "next minor",
+			want: semversion{Major: 0, Minor: 1, Patch: 0},
+		},
+		{
+			name: "next patch version of v0.2.2 is v0.2.3",
+			tags: []string{"gopls/0.1.1", "gopls/0.2.0", "gopls/0.2.1", "gopls/v0.2.2"},
+			bump: "next patch",
+			want: semversion{Major: 0, Minor: 2, Patch: 3},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			tools := NewFakeRepo(t, "tools")
+			commit := tools.Commit(map[string]string{
+				"go.mod": "module golang.org/x/tools\n",
+				"go.sum": "\n",
+			})
+
+			for _, tag := range tc.tags {
+				tools.Tag(tag, commit)
+			}
+
+			gerrit := NewFakeGerrit(t, tools)
+
+			tasks := &ReleaseGoplsTasks{
+				Gerrit: gerrit,
+			}
+
+			got, err := tasks.interpretNextRelease(&workflow.TaskContext{Context: context.Background(), Logger: &testLogger{t, ""}}, tc.bump)
+			if err != nil {
+				t.Fatalf("interpretNextRelease(%q) should not return error, but return %v", tc.bump, err)
+			}
+			if tc.want != got {
+				t.Errorf("interpretNextRelease(%q) = %v, want %v", tc.bump, tc.want, got)
+			}
+		})
+	}
+}
+
 func TestPossibleGoplsVersions(t *testing.T) {
 	tests := []struct {
 		name string
@@ -367,7 +423,7 @@ func TestNextPrerelease(t *testing.T) {
 			if !ok {
 				t.Fatalf("parseSemver(%q) should success", tc.version)
 			}
-			got, err := tasks.nextPrerelease(&workflow.TaskContext{Context: ctx, Logger: &testLogger{t, ""}}, semv)
+			got, err := tasks.nextPrereleaseVersion(&workflow.TaskContext{Context: ctx, Logger: &testLogger{t, ""}}, semv)
 			if err != nil {
 				t.Fatalf("nextPrerelease(%q) should not return error: %v", tc.version, err)
 			}
@@ -501,30 +557,32 @@ func TestGoplsPrereleaseFlow(t *testing.T) {
 		// For each entry, a new commit is created, and if the entry is
 		// non empty that commit is tagged with the entry value.
 		commitTags []string
-		config     string
-		semv       semversion
+		// If set, create the release branch before starting the workflow.
+		createBranch bool
+		config       string
+		semv         semversion
 		// fields below are the desired states.
 		wantVersion string
 		wantConfig  string
 		wantCommits int
 	}{
 		{
-			name: "update all three file through two commits",
-			// create release branch with one commit without any tag.
-			commitTags:  []string{""},
-			config:      " ",
-			semv:        semversion{Major: 0, Minor: 1, Patch: 0},
-			wantVersion: "v0.1.0-pre.1",
+			name:         "update all three file through two commits",
+			commitTags:   []string{"gopls/v0.0.0"},
+			createBranch: true,
+			config:       " ",
+			semv:         semversion{Major: 0, Minor: 1, Patch: 0},
+			wantVersion:  "v0.1.0-pre.1",
 			wantConfig: `issuerepo: golang/go
 branch: gopls-release-branch.0.1
 parent-branch: master
 `,
 			wantCommits: 2,
 		},
 		{
-			name: "codereview.cfg already have expected content, update go.mod and go.sum with one commit",
-			// create release branch with one commit without any tag.
-			commitTags: []string{""},
+			name:         "codereview.cfg already have expected content, update go.mod and go.sum with one commit",
+			commitTags:   []string{"gopls/v0.0.0"},
+			createBranch: true,
 			config: `issuerepo: golang/go
 branch: gopls-release-branch.0.1
 parent-branch: master
@@ -538,24 +596,25 @@ parent-branch: master
 			wantCommits: 1,
 		},
 		{
-			name:        "create the branch for minor version",
-			commitTags:  nil, // no release branch
-			config:      ` `,
-			semv:        semversion{Major: 0, Minor: 12, Patch: 0},
-			wantVersion: "v0.12.0-pre.1",
+			name:         "create the branch for minor version",
+			commitTags:   []string{"gopls/v0.11.0"},
+			createBranch: false,
+			config:       ` `,
+			semv:         semversion{Major: 0, Minor: 12, Patch: 0},
+			wantVersion:  "v0.12.0-pre.1",
 			wantConfig: `issuerepo: golang/go
 branch: gopls-release-branch.0.12
 parent-branch: master
 `,
 			wantCommits: 2,
 		},
 		{
-			name: "workflow should increment the pre-release number to 4",
-			// create release branch with three commits with tags.
-			commitTags:  []string{"gopls/v0.8.3-pre.1", "gopls/v0.8.3-pre.2", "gopls/v0.8.3-pre.3"},
-			config:      " ",
-			semv:        semversion{Major: 0, Minor: 8, Patch: 3},
-			wantVersion: "v0.8.3-pre.4",
+			name:         "workflow should increment the pre-release number to 4",
+			commitTags:   []string{"gopls/v0.8.2", "gopls/v0.8.3-pre.1", "gopls/v0.8.3-pre.2", "gopls/v0.8.3-pre.3"},
+			createBranch: true,
+			config:       " ",
+			semv:         semversion{Major: 0, Minor: 8, Patch: 3},
+			wantVersion:  "v0.8.3-pre.4",
 			wantConfig: `issuerepo: golang/go
 branch: gopls-release-branch.0.8
 parent-branch: master
@@ -565,7 +624,7 @@ parent-branch: master
 	}
 
 	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
+		runTestWithInput := func(input map[string]any) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
@@ -575,21 +634,14 @@ parent-branch: master
 				"gopls/go.sum":   "\n",
 				"codereview.cfg": tc.config,
 			})
-			// These tags make sure the input version is the next version of some
-			// released version, in order to pass the version validation step.
-			tools.Tag(fmt.Sprintf("gopls/v%v.%v.%v", tc.semv.Major, tc.semv.Minor, tc.semv.Patch-1), beforeHead)
-			tools.Tag(fmt.Sprintf("gopls/v%v.%v.%v", tc.semv.Major, tc.semv.Minor-1, tc.semv.Patch), beforeHead)
-			tools.Tag(fmt.Sprintf("gopls/v%v.%v.%v", tc.semv.Major-1, tc.semv.Minor, tc.semv.Patch), beforeHead)
-
-			// Create the release branch and make a few commits to the release branch.
+			// Create the release branch and make a few commits to the master branch.
 			// Var beforeHead is used to track the commit of release branch's head
 			// before trigger the gopls pre-release run. If we do not need to create a
 			// release branch, beforeHead will point to the initial commit in the
 			// master branch.
 			if len(tc.commitTags) != 0 {
-				tools.Branch(goplsReleaseBranchName(tc.semv), beforeHead)
 				for i, tag := range tc.commitTags {
-					commit := tools.CommitOnBranch(goplsReleaseBranchName(tc.semv), map[string]string{
+					commit := tools.CommitOnBranch("master", map[string]string{
 						"README.md": fmt.Sprintf("THIS IS READ ME FOR %v.", i),
 					})
 					beforeHead = commit
@@ -599,6 +651,10 @@ parent-branch: master
 				}
 			}
 
+			if tc.createBranch {
+				tools.Branch(goplsReleaseBranchName(tc.semv), beforeHead)
+			}
+
 			gerrit := NewFakeGerrit(t, tools)
 
 			// fakeGo handles multiple arguments in gopls pre-release flow.
@@ -664,10 +720,7 @@ esac`, tc.wantVersion)
 			}
 
 			wd := tasks.NewPrereleaseDefinition()
-			w, err := workflow.Start(wd, map[string]interface{}{
-				reviewersParam.Name: []string(nil),
-				"version":           fmt.Sprintf("v%v.%v.%v", tc.semv.Major, tc.semv.Minor, tc.semv.Patch),
-			})
+			w, err := workflow.Start(wd, input)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -741,6 +794,24 @@ esac`, tc.wantVersion)
 			if info.Revision != afterHead {
 				t.Errorf("the pre-release tag points to commit %s, should point to the head commit of release branch %s", info.Revision, afterHead)
 			}
+		}
+		t.Run("manual input version: "+tc.name, func(t *testing.T) {
+			runTestWithInput(map[string]any{
+				reviewersParam.Name:           []string(nil),
+				"explicit version (optional)": fmt.Sprintf("v%v.%v.%v", tc.semv.Major, tc.semv.Minor, tc.semv.Patch),
+				"next version":                "use explicit version",
+			})
+		})
+		versionBump := "next patch"
+		if tc.semv.Patch == 0 {
+			versionBump = "next minor"
+		}
+		t.Run("interpret version "+versionBump+" : "+tc.name, func(t *testing.T) {
+			runTestWithInput(map[string]any{
+				reviewersParam.Name:           []string(nil),
+				"explicit version (optional)": "",
+				"next version":                versionBump,
+			})
 		})
 	}
 }