internal/workflow: fix sub-workflow prefix for tasks added via expansion

Tasks added by expansions, in contrast to all other tasks, didn't handle
the sub-workflow prefix. A while back I looked into it briefly, and saw
that shallowClone didn't clone that one field, so figured CL 546235
would fix it.

It didn't, and now it's apparent to me why not. The expansion always
gets a copy of the top-level workflow definition, even if it was made
by a task with some prefix. Modify addExpansion to record not just that
a given task is an expansion, but also the workflow prefix at that time.

We also have a test that makes it easy to verify this works now.
Keep shallowClone as is, to match the behavior implied by its name,
even though by now the namePrefix it clones gets overwritten anyway.

For golang/go#70249.

Change-Id: Ib1cb34ef121baf946fe6f2500c4bf1611aaa6db7
Reviewed-on: https://go-review.googlesource.com/c/build/+/626336
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>

Author: dmitshur

internal/workflow/workflow.go

@@ -422,6 +422,9 @@ func addAction(d *Definition, name string, f interface{}, inputs []metaValue, op
 func addExpansion[O1 any](d *Definition, name string, f interface{}, inputs []metaValue, opts []TaskOption) *expansionResult[O1] {
 	td := addFunc(d, name, f, inputs, opts)
 	td.isExpansion = true
+	// Also record the workflow name prefix at the time the expansion is added.
+	// It'll be accessed later, when starting to run this expansion.
+	td.namePrefix = d.namePrefix
 	return &expansionResult[O1]{td}
 }
 
@@ -600,6 +603,7 @@ type Logger interface {
 type taskDefinition struct {
 	name        string
 	isExpansion bool
+	namePrefix  string // Workflow name prefix; applies only when isExpansion is true.
 	args        []metaValue
 	deps        []Dependency
 	f           interface{}
@@ -845,6 +849,7 @@ func (w *Workflow) Run(ctx context.Context, listener Listener) (map[string]inter
 				if task.def.isExpansion {
 					runningExpansion = true
 					defCopy := w.def.shallowClone()
+					defCopy.namePrefix = task.def.namePrefix
 					go func() { stateChan <- runExpansion(defCopy, taskCopy, args) }()
 				} else {
 					go func() { stateChan <- runTask(ctx, w.ID, listener, taskCopy, args) }()

internal/workflow/workflow_test.go

@@ -361,24 +361,24 @@ func TestManualRetryMultipleExpansions(t *testing.T) {
 
 	w := startWorkflow(t, wd, nil)
 	listener := &errorListener{
-		taskName: "work 1",
+		taskName: "sub1: work 1",
 		callback: func(string) {
 			go func() {
 				retried[0]++
-				err := w.RetryTask(context.Background(), "work 1")
+				err := w.RetryTask(context.Background(), "sub1: work 1")
 				if err != nil {
-					t.Errorf(`RetryTask("work 1") failed: %v`, err)
+					t.Errorf(`RetryTask("sub1: work 1") failed: %v`, err)
 				}
 			}()
 		},
 		Listener: &errorListener{
-			taskName: "work 2",
+			taskName: "sub2: work 2",
 			callback: func(string) {
 				go func() {
 					retried[1]++
-					err := w.RetryTask(context.Background(), "work 2")
+					err := w.RetryTask(context.Background(), "sub2: work 2")
 					if err != nil {
-						t.Errorf(`RetryTask("work 2") failed: %v`, err)
+						t.Errorf(`RetryTask("sub2: work 2") failed: %v`, err)
 					}
 				}()
 			},