internal/task: ensure go is in PATH when generating telemetry config

This changed caused a test failure due to use of ExecOpts.Path with the
fake buildlet. Turn off that check to get the test passing, leaving a
TODO to implement proper Path support.

Also, restrict TestTagTelemetry to builders that actually have shell
scripting support.

Change-Id: Ic9ffbc8979b468d0338fb46ec26696c014a26fb0
Reviewed-on: https://go-review.googlesource.com/c/build/+/522897
Reviewed-by: Heschi Kreinick <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
Auto-Submit: Robert Findley <[email protected]>

Author: findleyr

internal/task/fakes.go

@@ -181,9 +181,10 @@ func (b *fakeBuildlet) Close() error {
 }
 
 func (b *fakeBuildlet) Exec(ctx context.Context, cmd string, opts buildlet.ExecOpts) (remoteErr error, execErr error) {
-	if opts.Path != nil {
-		return nil, fmt.Errorf("opts.Path option is set, but fakeBuildlet doesn't support it")
-	} else if opts.OnStartExec != nil {
+	// TODO: add support for opts.Path. Previously, setting opts.Path would cause
+	// an error here, but that caused unnecessary failures in tests that use mock
+	// execution.
+	if opts.OnStartExec != nil {
 		return nil, fmt.Errorf("opts.OnStartExec option is set, but fakeBuildlet doesn't support it")
 	}
 	b.logf("exec %v %v\n\twd %q env %v", cmd, opts.Args, opts.Dir, opts.ExtraEnv)

internal/task/tagtelemetry.go

@@ -92,10 +92,11 @@ func (t *TagTelemetryTasks) GenerateConfig(ctx *wf.TaskContext, reviewers []stri
 
 	logWriter := &LogWriter{Logger: ctx}
 	go logWriter.Run(ctx)
-	remoteErr, execErr := bc.Exec(ctx, "go/bin/go", buildlet.ExecOpts{
+	remoteErr, execErr := bc.Exec(ctx, "./go/bin/go", buildlet.ExecOpts{
 		Dir:      "telemetry",
 		Args:     []string{"run", "./internal/configgen", "-w"},
 		Output:   logWriter,
+		Path:     []string{"$WORKDIR/go/bin", "$PATH"},
 		ExtraEnv: []string{"GO_DISABLE_OUTBOUND_NETWORK=0"},
 	})
 	if execErr != nil {

internal/task/tagtelemetry_test.go

@@ -16,6 +16,8 @@ import (
 )
 
 func TestTagTelemetry(t *testing.T) {
+	mustHaveShell(t)
+
 	tests := []struct {
 		label           string
 		tags            []string

internal/task/tagx_test.go

@@ -344,10 +344,16 @@ type tagXTestDeps struct {
 	tagXTasks *TagXReposTasks
 }
 
-func newTagXTestDeps(t *testing.T, dashboardStatus string, repos ...*FakeRepo) *tagXTestDeps {
+// mustHaveShell skips if the current environment doesn't support shell
+// scripting (/bin/bash).
+func mustHaveShell(t *testing.T) {
 	if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
 		t.Skip("Requires bash shell scripting support.")
 	}
+}
+
+func newTagXTestDeps(t *testing.T, dashboardStatus string, repos ...*FakeRepo) *tagXTestDeps {
+	mustHaveShell(t)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)