Add error checking in tests and enable errcheck there (#1980)

## Changes Fix all errcheck-found issues in tests and test helpers. Mostly this done by adding require.NoError(t, err), sometimes panic() where t object is not available). Initial change is obtained with aider+claude, then manually reviewed and cleaned up. ## Tests Existing tests.
databricks · Dec 9, 2024 · 1b2be1b · 1b2be1b
1 parent e0d54f0
commit 1b2be1b
Show file tree

Hide file tree

Showing 41 changed files with 233 additions and 143 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -2,9 +2,7 @@ linters:
   disable-all: true
   enable:
     - bodyclose
-    # errcheck and govet are part of default setup and should be included but give too many errors now
-    # once errors are fixed, they should be enabled here:
-    #- errcheck
+    - errcheck
     - gosimple
     #- govet
     - ineffassign
@@ -20,3 +18,7 @@ linters-settings:
         replacement: 'any'
 issues:
   exclude-dirs-use-default: false  # recommended by docs https://golangci-lint.run/usage/false-positives/
+  exclude-rules:
+    - path-except: (_test\.go|internal)
+      linters:
+        - errcheck
diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go
@@ -110,7 +110,8 @@ func TestInitializeURLs(t *testing.T) {
 		"dashboard1":       "https://mycompany.databricks.com/dashboardsv3/01ef8d56871e1d50ae30ce7375e42478/published?o=123456",
 	}
 
-	initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/")
+	err := initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/")
+	require.NoError(t, err)
 
 	for _, group := range b.Config.Resources.AllResources() {
 		for key, r := range group.Resources {
@@ -133,7 +134,8 @@ func TestInitializeURLsWithoutOrgId(t *testing.T) {
 		},
 	}
 
-	initializeForWorkspace(b, "123456", "https://adb-123456.azuredatabricks.net/")
+	err := initializeForWorkspace(b, "123456", "https://adb-123456.azuredatabricks.net/")
+	require.NoError(t, err)
 
 	require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL)
 }
diff --git a/bundle/config/mutator/resolve_resource_references_test.go b/bundle/config/mutator/resolve_resource_references_test.go
@@ -108,7 +108,8 @@ func TestNoLookupIfVariableIsSet(t *testing.T) {
 	m := mocks.NewMockWorkspaceClient(t)
 	b.SetWorkpaceClient(m.WorkspaceClient)
 
-	b.Config.Variables["my-cluster-id"].Set("random value")
+	err := b.Config.Variables["my-cluster-id"].Set("random value")
+	require.NoError(t, err)
 
 	diags := bundle.Apply(context.Background(), b, ResolveResourceReferences())
 	require.NoError(t, diags.Error())

diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go
@@ -28,7 +28,8 @@ import (
 func touchNotebookFile(t *testing.T, path string) {
 	f, err := os.Create(path)
 	require.NoError(t, err)
-	f.WriteString("# Databricks notebook source\n")
+	_, err = f.WriteString("# Databricks notebook source\n")
+	require.NoError(t, err)
 	f.Close()
 }
 

diff --git a/bundle/config/resources_test.go b/bundle/config/resources_test.go
@@ -49,7 +49,8 @@ func TestCustomMarshallerIsImplemented(t *testing.T) {
 		// Eg: resource.Job implements MarshalJSON
 		v := reflect.Zero(vt.Elem()).Interface()
 		assert.NotPanics(t, func() {
-			json.Marshal(v)
+			_, err := json.Marshal(v)
+			assert.NoError(t, err)
 		}, "Resource %s does not have a custom marshaller", field.Name)
 
 		// Unmarshalling a *resourceStruct will panic if the resource does not have a custom unmarshaller
@@ -58,7 +59,8 @@ func TestCustomMarshallerIsImplemented(t *testing.T) {
 		// Eg: *resource.Job implements UnmarshalJSON
 		v = reflect.New(vt.Elem()).Interface()
 		assert.NotPanics(t, func() {
-			json.Unmarshal([]byte("{}"), v)
+			err := json.Unmarshal([]byte("{}"), v)
+			assert.NoError(t, err)
 		}, "Resource %s does not have a custom unmarshaller", field.Name)
 	}
 }

diff --git a/bundle/config/root_test.go b/bundle/config/root_test.go
@@ -100,7 +100,7 @@ func TestRootMergeTargetOverridesWithMode(t *testing.T) {
 			},
 		},
 	}
-	root.initializeDynamicValue()
+	require.NoError(t, root.initializeDynamicValue())
 	require.NoError(t, root.MergeTargetOverrides("development"))
 	assert.Equal(t, Development, root.Bundle.Mode)
 }
@@ -156,7 +156,7 @@ func TestRootMergeTargetOverridesWithVariables(t *testing.T) {
 			},
 		},
 	}
-	root.initializeDynamicValue()
+	require.NoError(t, root.initializeDynamicValue())
 	require.NoError(t, root.MergeTargetOverrides("development"))
 	assert.Equal(t, "bar", root.Variables["foo"].Default)
 	assert.Equal(t, "foo var", root.Variables["foo"].Description)

diff --git a/bundle/config/workspace_test.go b/bundle/config/workspace_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/databricks/cli/libs/databrickscfg"
 	"github.com/databricks/databricks-sdk-go/config"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func setupWorkspaceTest(t *testing.T) string {
@@ -42,11 +43,12 @@ func TestWorkspaceResolveProfileFromHost(t *testing.T) {
 		setupWorkspaceTest(t)
 
 		// This works if there is a config file with a matching profile.
-		databrickscfg.SaveToProfile(context.Background(), &config.Config{
+		err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
 			Profile: "default",
 			Host:    "https://abc.cloud.databricks.com",
 			Token:   "123",
 		})
+		require.NoError(t, err)
 
 		client, err := w.Client()
 		assert.NoError(t, err)
@@ -57,12 +59,13 @@ func TestWorkspaceResolveProfileFromHost(t *testing.T) {
 		home := setupWorkspaceTest(t)
 
 		// This works if there is a config file with a matching profile.
-		databrickscfg.SaveToProfile(context.Background(), &config.Config{
+		err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
 			ConfigFile: filepath.Join(home, "customcfg"),
 			Profile:    "custom",
 			Host:       "https://abc.cloud.databricks.com",
 			Token:      "123",
 		})
+		require.NoError(t, err)
 
 		t.Setenv("DATABRICKS_CONFIG_FILE", filepath.Join(home, "customcfg"))
 		client, err := w.Client()
@@ -90,55 +93,59 @@ func TestWorkspaceVerifyProfileForHost(t *testing.T) {
 		setupWorkspaceTest(t)
 
 		// This works if there is a config file with a matching profile.
-		databrickscfg.SaveToProfile(context.Background(), &config.Config{
+		err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
 			Profile: "abc",
 			Host:    "https://abc.cloud.databricks.com",
 		})
+		require.NoError(t, err)
 
-		_, err := w.Client()
+		_, err = w.Client()
 		assert.NoError(t, err)
 	})
 
 	t.Run("default config file with mismatch", func(t *testing.T) {
 		setupWorkspaceTest(t)
 
 		// This works if there is a config file with a matching profile.
-		databrickscfg.SaveToProfile(context.Background(), &config.Config{
+		err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
 			Profile: "abc",
 			Host:    "https://def.cloud.databricks.com",
 		})
+		require.NoError(t, err)
 
-		_, err := w.Client()
+		_, err = w.Client()
 		assert.ErrorContains(t, err, "config host mismatch")
 	})
 
 	t.Run("custom config file with match", func(t *testing.T) {
 		home := setupWorkspaceTest(t)
 
 		// This works if there is a config file with a matching profile.
-		databrickscfg.SaveToProfile(context.Background(), &config.Config{
+		err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
 			ConfigFile: filepath.Join(home, "customcfg"),
 			Profile:    "abc",
 			Host:       "https://abc.cloud.databricks.com",
 		})
+		require.NoError(t, err)
 
 		t.Setenv("DATABRICKS_CONFIG_FILE", filepath.Join(home, "customcfg"))
-		_, err := w.Client()
+		_, err = w.Client()
 		assert.NoError(t, err)
 	})
 
 	t.Run("custom config file with mismatch", func(t *testing.T) {
 		home := setupWorkspaceTest(t)
 
 		// This works if there is a config file with a matching profile.
-		databrickscfg.SaveToProfile(context.Background(), &config.Config{
+		err := databrickscfg.SaveToProfile(context.Background(), &config.Config{
 			ConfigFile: filepath.Join(home, "customcfg"),
 			Profile:    "abc",
 			Host:       "https://def.cloud.databricks.com",
 		})
+		require.NoError(t, err)
 
 		t.Setenv("DATABRICKS_CONFIG_FILE", filepath.Join(home, "customcfg"))
-		_, err := w.Client()
+		_, err = w.Client()
 		assert.ErrorContains(t, err, "config host mismatch")
 	})
 }
diff --git a/bundle/internal/bundletest/location.go b/bundle/internal/bundletest/location.go
@@ -10,7 +10,7 @@ import (
 // with the path it is loaded from.
 func SetLocation(b *bundle.Bundle, prefix string, locations []dyn.Location) {
 	start := dyn.MustPathFromString(prefix)
-	b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) {
+	err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) {
 		return dyn.Walk(root, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
 			// If the path has the given prefix, set the location.
 			if p.HasPrefix(start) {
@@ -27,4 +27,7 @@ func SetLocation(b *bundle.Bundle, prefix string, locations []dyn.Location) {
 			return v, dyn.ErrSkip
 		})
 	})
+	if err != nil {
+		panic("Mutate() failed: " + err.Error())
+	}
 }
diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go
@@ -489,7 +489,8 @@ func TestRenderSummaryTemplate_nilBundle(t *testing.T) {
 	err := renderSummaryHeaderTemplate(writer, nil)
 	require.NoError(t, err)
 
-	io.WriteString(writer, buildTrailer(nil))
+	_, err = io.WriteString(writer, buildTrailer(nil))
+	require.NoError(t, err)
 
 	assert.Equal(t, "Validation OK!\n", writer.String())
 }

diff --git a/bundle/run/job_test.go b/bundle/run/job_test.go
@@ -42,15 +42,17 @@ func TestConvertPythonParams(t *testing.T) {
 	opts := &Options{
 		Job: JobOptions{},
 	}
-	runner.convertPythonParams(opts)
+	err := runner.convertPythonParams(opts)
+	require.NoError(t, err)
 	require.NotContains(t, opts.Job.notebookParams, "__python_params")
 
 	opts = &Options{
 		Job: JobOptions{
 			pythonParams: []string{"param1", "param2", "param3"},
 		},
 	}
-	runner.convertPythonParams(opts)
+	err = runner.convertPythonParams(opts)
+	require.NoError(t, err)
 	require.Contains(t, opts.Job.notebookParams, "__python_params")
 	require.Equal(t, opts.Job.notebookParams["__python_params"], `["param1","param2","param3"]`)
 }

diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go
@@ -31,22 +31,25 @@ func TestGetWorkspaceAuthStatus(t *testing.T) {
 
 	cmd.Flags().String("host", "", "")
 	cmd.Flags().String("profile", "", "")
-	cmd.Flag("profile").Value.Set("my-profile")
+	err := cmd.Flag("profile").Value.Set("my-profile")
+	require.NoError(t, err)
 	cmd.Flag("profile").Changed = true
 
 	cfg := &config.Config{
 		Profile: "my-profile",
 	}
 	m.WorkspaceClient.Config = cfg
 	t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli")
-	config.ConfigAttributes.Configure(cfg)
+	err = config.ConfigAttributes.Configure(cfg)
+	require.NoError(t, err)
 
 	status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
-		config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
+		err := config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
 			"host":      "https://test.com",
 			"token":     "test-token",
 			"auth_type": "azure-cli",
 		})
+		require.NoError(t, err)
 		return cfg, false, nil
 	})
 	require.NoError(t, err)
@@ -81,18 +84,20 @@ func TestGetWorkspaceAuthStatusError(t *testing.T) {
 
 	cmd.Flags().String("host", "", "")
 	cmd.Flags().String("profile", "", "")
-	cmd.Flag("profile").Value.Set("my-profile")
+	err := cmd.Flag("profile").Value.Set("my-profile")
+	require.NoError(t, err)
 	cmd.Flag("profile").Changed = true
 
 	cfg := &config.Config{
 		Profile: "my-profile",
 	}
 	m.WorkspaceClient.Config = cfg
 	t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli")
-	config.ConfigAttributes.Configure(cfg)
+	err = config.ConfigAttributes.Configure(cfg)
+	require.NoError(t, err)
 
 	status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
-		config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
+		err = config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
 			"host":      "https://test.com",
 			"token":     "test-token",
 			"auth_type": "azure-cli",
@@ -128,18 +133,20 @@ func TestGetWorkspaceAuthStatusSensitive(t *testing.T) {
 
 	cmd.Flags().String("host", "", "")
 	cmd.Flags().String("profile", "", "")
-	cmd.Flag("profile").Value.Set("my-profile")
+	err := cmd.Flag("profile").Value.Set("my-profile")
+	require.NoError(t, err)
 	cmd.Flag("profile").Changed = true
 
 	cfg := &config.Config{
 		Profile: "my-profile",
 	}
 	m.WorkspaceClient.Config = cfg
 	t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli")
-	config.ConfigAttributes.Configure(cfg)
+	err = config.ConfigAttributes.Configure(cfg)
+	require.NoError(t, err)
 
 	status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
-		config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
+		err = config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
 			"host":      "https://test.com",
 			"token":     "test-token",
 			"auth_type": "azure-cli",
@@ -171,21 +178,23 @@ func TestGetAccountAuthStatus(t *testing.T) {
 
 	cmd.Flags().String("host", "", "")
 	cmd.Flags().String("profile", "", "")
-	cmd.Flag("profile").Value.Set("my-profile")
+	err := cmd.Flag("profile").Value.Set("my-profile")
+	require.NoError(t, err)
 	cmd.Flag("profile").Changed = true
 
 	cfg := &config.Config{
 		Profile: "my-profile",
 	}
 	m.AccountClient.Config = cfg
 	t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli")
-	config.ConfigAttributes.Configure(cfg)
+	err = config.ConfigAttributes.Configure(cfg)
+	require.NoError(t, err)
 
 	wsApi := m.GetMockWorkspacesAPI()
 	wsApi.EXPECT().List(mock.Anything).Return(nil, nil)
 
 	status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
-		config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
+		err = config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{
 			"account_id": "test-account-id",
 			"username":   "test-user",
 			"host":       "https://test.com",