fix unit tests

coder · AbhineetJain · Jun 22, 2022 · May 26, 2022 · Jun 17, 2022 · Jun 17, 2022
commit 6b84a730f7fcdb0dcee93f0fefee86d7052ac17d
diff --git a/cli/server.go b/cli/server.go
@@ -376,7 +376,6 @@ func server() *cobra.Command {
 			shutdownConnsCtx, shutdownConns := context.WithCancel(cmd.Context())
 			defer shutdownConns()
 			go func() {
-				defer close(errCh)
 				server := http.Server{
 					// These errors are typically noise like "TLS: EOF". Vault does similar:
 					// https://github.com/hashicorp/vault/blob/e2490059d0711635e529a4efcbaa1b26998d6e1c/command/server.go#L2714
@@ -590,7 +589,7 @@ func newProvisionerDaemon(ctx context.Context, coderAPI *coderd.API,
 			CachePath: cacheDir,
 			Logger:    logger,
 		})
-		if err != nil {
+		if err != nil && !xerrors.Is(err, context.Canceled) {
 			errChan <- err
 		}
 	}()

diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go
@@ -112,7 +112,12 @@ func versionFromBinaryPath(ctx context.Context, binaryPath string) (*version.Ver
 	cmd := exec.CommandContext(ctx, binaryPath, "version", "-json")
 	out, err := cmd.Output()
 	if err != nil {
-		return nil, err
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		default:
+			return nil, err
+		}
 	}
 	vj := tfjson.VersionOutput{}
 	err = json.Unmarshal(out, &vj)

diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go
@@ -43,10 +43,10 @@ type ServeOptions struct {
 	Logger     slog.Logger
 }
 
-func getAbsoluteBinaryPath(ctx context.Context) (string, bool) {
+func absoluteBinaryPath(ctx context.Context) (string, error) {
 	binaryPath, err := safeexec.LookPath("terraform")
 	if err != nil {
-		return "", false
+		return "", xerrors.Errorf("Terraform binary not found: %w", err)
 	}
 
 	// If the "coder" binary is in the same directory as
@@ -56,30 +56,32 @@ func getAbsoluteBinaryPath(ctx context.Context) (string, bool) {
 	// to execute this properly!
 	absoluteBinary, err := filepath.Abs(binaryPath)
 	if err != nil {
-		return "", false
+		return "", xerrors.Errorf("Terraform binary absolute path not found: %w", err)
 	}
 
 	// Checking the installed version of Terraform.
 	version, err := versionFromBinaryPath(ctx, absoluteBinary)
 	if err != nil {
-		return "", false
+		return "", xerrors.Errorf("Terraform binary get version failed: %w", err)
 	}
 
 	if version.LessThan(minTerraformVersion) || version.GreaterThanOrEqual(maxTerraformVersion) {
-		return "", false
+		return "", xerrors.Errorf("Terraform binary minor version mismatch.")
 	}
 
-	return absoluteBinary, true
+	return absoluteBinary, nil
 }
 
 // Serve starts a dRPC server on the provided transport speaking Terraform provisioner.
 func Serve(ctx context.Context, options *ServeOptions) error {
 	if options.BinaryPath == "" {
-		absoluteBinary, ok := getAbsoluteBinaryPath(ctx)
+		absoluteBinary, err := absoluteBinaryPath(ctx)
+
+		if err != nil {
+			if xerrors.Is(err, context.Canceled) {
+				return xerrors.Errorf("absolute binary context canceled: %w", err)
+			}
 
-		if ok {
-			options.BinaryPath = absoluteBinary
-		} else {
 			installer := &releases.ExactVersion{
 				InstallDir: options.CachePath,
 				Product:    product.Terraform,
@@ -91,6 +93,8 @@ func Serve(ctx context.Context, options *ServeOptions) error {
 				return xerrors.Errorf("install terraform: %w", err)
 			}
 			options.BinaryPath = execPath
+		} else {
+			options.BinaryPath = absoluteBinary
 		}
 	}
 	return provisionersdk.Serve(ctx, &server{

diff --git a/provisioner/terraform/serve_internal_test.go b/provisioner/terraform/serve_internal_test.go
@@ -10,42 +10,43 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/xerrors"
 )
 
 // nolint:paralleltest
-func Test_getAbsoluteBinaryPath(t *testing.T) {
+func Test_absoluteBinaryPath(t *testing.T) {
 	type args struct {
 		ctx context.Context
 	}
 	tests := []struct {
 		name             string
 		args             args
 		terraformVersion string
-		expectedOk       bool
+		expectedErr      error
 	}{
 		{
 			name:             "TestCorrectVersion",
 			args:             args{ctx: context.Background()},
 			terraformVersion: "1.1.9",
-			expectedOk:       true,
+			expectedErr:      nil,
 		},
 		{
 			name:             "TestOldVersion",
 			args:             args{ctx: context.Background()},
 			terraformVersion: "1.0.9",
-			expectedOk:       false,
+			expectedErr:      xerrors.Errorf("Terraform binary minor version mismatch."),
 		},
 		{
 			name:             "TestNewVersion",
 			args:             args{ctx: context.Background()},
 			terraformVersion: "1.2.9",
-			expectedOk:       false,
+			expectedErr:      xerrors.Errorf("Terraform binary minor version mismatch."),
 		},
 		{
 			name:             "TestMalformedVersion",
 			args:             args{ctx: context.Background()},
 			terraformVersion: "version",
-			expectedOk:       false,
+			expectedErr:      xerrors.Errorf("Terraform binary get version failed: Malformed version: version"),
 		},
 	}
 	// nolint:paralleltest
@@ -80,16 +81,22 @@ func Test_getAbsoluteBinaryPath(t *testing.T) {
 			t.Setenv("PATH", strings.Join([]string{tempDir, pathVariable}, ":"))
 
 			var expectedAbsoluteBinary string
-			if tt.expectedOk {
+			if tt.expectedErr == nil {
 				expectedAbsoluteBinary = filepath.Join(tempDir, "terraform")
 			}
 
-			actualAbsoluteBinary, actualOk := getAbsoluteBinaryPath(tt.args.ctx)
+			actualAbsoluteBinary, actualErr := absoluteBinaryPath(tt.args.ctx)
 			if actualAbsoluteBinary != expectedAbsoluteBinary {
 				t.Errorf("getAbsoluteBinaryPath() absoluteBinaryPath, actual = %v, expected %v", actualAbsoluteBinary, expectedAbsoluteBinary)
 			}
-			if actualOk != tt.expectedOk {
-				t.Errorf("getAbsoluteBinaryPath() ok, actual = %v, expected %v", actualOk, tt.expectedOk)
+			if tt.expectedErr == nil {
+				if actualErr != nil {
+					t.Errorf("getAbsoluteBinaryPath() error, actual = %v, expected %v", actualErr.Error(), tt.expectedErr)
+				}
+			} else {
+				if actualErr.Error() != tt.expectedErr.Error() {
+					t.Errorf("getAbsoluteBinaryPath() error, actual = %v, expected %v", actualErr.Error(), tt.expectedErr.Error())
+				}
 			}
 		})
 	}