Skip to content

Commit

Permalink
Move test helpers from internal to acc and testutil (#2008)
Browse files Browse the repository at this point in the history
## Changes

This change moves fixture helpers to `internal/acc/fixtures.go`. These
helpers create an ephemeral path or resource for the duration of a test.
Call sites are updated to use `acc.WorkspaceTest()` to construct a
workspace-focused test wrapper as needed.

This change also moves the `GetNodeTypeID()` function to `testutil`.

## Tests

n/a
  • Loading branch information
pietern authored Dec 12, 2024
1 parent e472b5d commit 61b0c59
Show file tree
Hide file tree
Showing 26 changed files with 309 additions and 380 deletions.
133 changes: 133 additions & 0 deletions internal/acc/fixtures.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package acc

import (
"fmt"

"github.com/databricks/cli/internal/testutil"
"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/catalog"
"github.com/databricks/databricks-sdk-go/service/files"
"github.com/databricks/databricks-sdk-go/service/workspace"
"github.com/stretchr/testify/require"
)

func TemporaryWorkspaceDir(t *WorkspaceT, name ...string) string {
ctx := t.ctx
me, err := t.W.CurrentUser.Me(ctx)
require.NoError(t, err)

// Prefix the name with "integration-test-" to make it easier to identify.
name = append([]string{"integration-test-"}, name...)
basePath := fmt.Sprintf("/Users/%s/%s", me.UserName, testutil.RandomName(name...))

t.Logf("Creating workspace directory %s", basePath)
err = t.W.Workspace.MkdirsByPath(ctx, basePath)
require.NoError(t, err)

// Remove test directory on test completion.
t.Cleanup(func() {
t.Logf("Removing workspace directory %s", basePath)
err := t.W.Workspace.Delete(ctx, workspace.Delete{
Path: basePath,
Recursive: true,
})
if err == nil || apierr.IsMissing(err) {
return
}
t.Logf("Unable to remove temporary workspace directory %s: %#v", basePath, err)
})

return basePath
}

func TemporaryDbfsDir(t *WorkspaceT, name ...string) string {
ctx := t.ctx

// Prefix the name with "integration-test-" to make it easier to identify.
name = append([]string{"integration-test-"}, name...)
path := fmt.Sprintf("/tmp/%s", testutil.RandomName(name...))

t.Logf("Creating DBFS directory %s", path)
err := t.W.Dbfs.MkdirsByPath(ctx, path)
require.NoError(t, err)

t.Cleanup(func() {
t.Logf("Removing DBFS directory %s", path)
err := t.W.Dbfs.Delete(ctx, files.Delete{
Path: path,
Recursive: true,
})
if err == nil || apierr.IsMissing(err) {
return
}
t.Logf("Unable to remove temporary DBFS directory %s: %#v", path, err)
})

return path
}

func TemporaryRepo(t *WorkspaceT, url string) string {
ctx := t.ctx
me, err := t.W.CurrentUser.Me(ctx)
require.NoError(t, err)

// Prefix the path with "integration-test-" to make it easier to identify.
path := fmt.Sprintf("/Repos/%s/%s", me.UserName, testutil.RandomName("integration-test-"))

t.Logf("Creating repo: %s", path)
resp, err := t.W.Repos.Create(ctx, workspace.CreateRepoRequest{
Url: url,
Path: path,
Provider: "gitHub",
})
require.NoError(t, err)

t.Cleanup(func() {
t.Logf("Removing repo: %s", path)
err := t.W.Repos.Delete(ctx, workspace.DeleteRepoRequest{
RepoId: resp.Id,
})
if err == nil || apierr.IsMissing(err) {
return
}
t.Logf("Unable to remove repo %s: %#v", path, err)
})

return path
}

// Create a new Unity Catalog volume in a catalog called "main" in the workspace.
func TemporaryVolume(t *WorkspaceT) string {
ctx := t.ctx
w := t.W

// Create a schema
schema, err := w.Schemas.Create(ctx, catalog.CreateSchema{
CatalogName: "main",
Name: testutil.RandomName("test-schema-"),
})
require.NoError(t, err)
t.Cleanup(func() {
err := w.Schemas.Delete(ctx, catalog.DeleteSchemaRequest{
FullName: schema.FullName,
})
require.NoError(t, err)
})

// Create a volume
volume, err := w.Volumes.Create(ctx, catalog.CreateVolumeRequestContent{
CatalogName: "main",
SchemaName: schema.Name,
Name: "my-volume",
VolumeType: catalog.VolumeTypeManaged,
})
require.NoError(t, err)
t.Cleanup(func() {
err := w.Volumes.Delete(ctx, catalog.DeleteVolumeRequest{
Name: volume.FullName,
})
require.NoError(t, err)
})

return fmt.Sprintf("/Volumes/%s/%s/%s", "main", schema.Name, volume.Name)
}
30 changes: 0 additions & 30 deletions internal/acc/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@ package acc

import (
"context"
"fmt"
"os"

"github.com/databricks/cli/internal/testutil"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/databricks/databricks-sdk-go/service/workspace"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -97,30 +94,3 @@ func (t *WorkspaceT) RunPython(code string) (string, error) {
require.True(t, ok, "unexpected type %T", results.Data)
return output, nil
}

func (t *WorkspaceT) TemporaryWorkspaceDir(name ...string) string {
ctx := context.Background()
me, err := t.W.CurrentUser.Me(ctx)
require.NoError(t, err)

basePath := fmt.Sprintf("/Users/%s/%s", me.UserName, testutil.RandomName(name...))

t.Logf("Creating %s", basePath)
err = t.W.Workspace.MkdirsByPath(ctx, basePath)
require.NoError(t, err)

// Remove test directory on test completion.
t.Cleanup(func() {
t.Logf("Removing %s", basePath)
err := t.W.Workspace.Delete(ctx, workspace.Delete{
Path: basePath,
Recursive: true,
})
if err == nil || apierr.IsMissing(err) {
return
}
t.Logf("Unable to remove temporary workspace directory %s: %#v", basePath, err)
})

return basePath
}
10 changes: 3 additions & 7 deletions internal/bundle/artifacts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/libraries"
"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/internal/testcli"
"github.com/databricks/cli/internal/testutil"
Expand All @@ -34,12 +33,11 @@ func touchEmptyFile(t *testing.T, path string) {

func TestAccUploadArtifactFileToCorrectRemotePath(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W
dir := t.TempDir()
whlPath := filepath.Join(dir, "dist", "test.whl")
touchEmptyFile(t, whlPath)

wsDir := internal.TemporaryWorkspaceDir(t, w)
wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-")

b := &bundle.Bundle{
BundleRootPath: dir,
Expand Down Expand Up @@ -99,12 +97,11 @@ func TestAccUploadArtifactFileToCorrectRemotePath(t *testing.T) {

func TestAccUploadArtifactFileToCorrectRemotePathWithEnvironments(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W
dir := t.TempDir()
whlPath := filepath.Join(dir, "dist", "test.whl")
touchEmptyFile(t, whlPath)

wsDir := internal.TemporaryWorkspaceDir(t, w)
wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-")

b := &bundle.Bundle{
BundleRootPath: dir,
Expand Down Expand Up @@ -164,13 +161,12 @@ func TestAccUploadArtifactFileToCorrectRemotePathWithEnvironments(t *testing.T)

func TestAccUploadArtifactFileToCorrectRemotePathForVolumes(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W

if os.Getenv("TEST_METASTORE_ID") == "" {
t.Skip("Skipping tests that require a UC Volume when metastore id is not set.")
}

volumePath := internal.TemporaryUcVolume(t, w)
volumePath := acc.TemporaryVolume(wt)

dir := t.TempDir()
whlPath := filepath.Join(dir, "dist", "test.whl")
Expand Down
5 changes: 2 additions & 3 deletions internal/bundle/basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@ import (
"path/filepath"
"testing"

"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/libs/env"
"github.com/databricks/cli/internal/testutil"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
)

func TestAccBasicBundleDeployWithFailOnActiveRuns(t *testing.T) {
ctx, _ := acc.WorkspaceTest(t)

nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV"))
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
root, err := initTestTemplate(t, ctx, "basic", map[string]any{
"unique_id": uniqueId,
Expand Down
11 changes: 5 additions & 6 deletions internal/bundle/bind_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"path/filepath"
"testing"

"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/internal/testcli"
"github.com/databricks/cli/internal/testutil"
Expand All @@ -22,9 +21,9 @@ func TestAccBindJobToExistingJob(t *testing.T) {
t.Log(env)

ctx, wt := acc.WorkspaceTest(t)
gt := &generateJobTest{T: t, w: wt.W}
gt := &generateJobTest{T: wt, w: wt.W}

nodeTypeId := internal.GetNodeTypeId(env)
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "basic", map[string]any{
"unique_id": uniqueId,
Expand Down Expand Up @@ -87,9 +86,9 @@ func TestAccAbortBind(t *testing.T) {
t.Log(env)

ctx, wt := acc.WorkspaceTest(t)
gt := &generateJobTest{T: t, w: wt.W}
gt := &generateJobTest{T: wt, w: wt.W}

nodeTypeId := internal.GetNodeTypeId(env)
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "basic", map[string]any{
"unique_id": uniqueId,
Expand Down Expand Up @@ -136,7 +135,7 @@ func TestAccGenerateAndBind(t *testing.T) {
t.Log(env)

ctx, wt := acc.WorkspaceTest(t)
gt := &generateJobTest{T: t, w: wt.W}
gt := &generateJobTest{T: wt, w: wt.W}

uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "with_includes", map[string]any{
Expand Down
4 changes: 1 addition & 3 deletions internal/bundle/clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ import (
"fmt"
"testing"

"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/env"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
Expand All @@ -20,7 +18,7 @@ func TestAccDeployBundleWithCluster(t *testing.T) {
t.Skip("Skipping test for AWS cloud because it is not permitted to create clusters")
}

nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV"))
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
root, err := initTestTemplate(t, ctx, "clusters", map[string]any{
"unique_id": uniqueId,
Expand Down
7 changes: 3 additions & 4 deletions internal/bundle/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ import (
"testing"

"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/internal/testcli"
"github.com/databricks/cli/libs/env"
"github.com/databricks/cli/internal/testutil"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/catalog"
Expand Down Expand Up @@ -133,7 +132,7 @@ func TestAccBundlePipelineDeleteWithoutAutoApprove(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W

nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV"))
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "deploy_then_remove_resources", map[string]any{
"unique_id": uniqueId,
Expand Down Expand Up @@ -219,7 +218,7 @@ properties such as the 'catalog' or 'storage' are changed:
func TestAccDeployBasicBundleLogs(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV"))
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
root, err := initTestTemplate(t, ctx, "basic", map[string]any{
"unique_id": uniqueId,
Expand Down
5 changes: 2 additions & 3 deletions internal/bundle/deploy_then_remove_resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import (
"path/filepath"
"testing"

"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/libs/env"
"github.com/databricks/cli/internal/testutil"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -17,7 +16,7 @@ func TestAccBundleDeployThenRemoveResources(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W

nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV"))
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "deploy_then_remove_resources", map[string]any{
"unique_id": uniqueId,
Expand Down
5 changes: 2 additions & 3 deletions internal/bundle/deploy_to_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@ import (
"fmt"
"testing"

"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/libs/env"
"github.com/databricks/cli/internal/testutil"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
)

func TestAccDeployBasicToSharedWorkspacePath(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV"))
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()

currentUser, err := wt.W.CurrentUser.Me(ctx)
Expand Down
3 changes: 1 addition & 2 deletions internal/bundle/deployment_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"testing"

"github.com/databricks/cli/bundle/deploy"
"github.com/databricks/cli/internal"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/internal/testutil"
"github.com/google/uuid"
Expand All @@ -21,7 +20,7 @@ func TestAccFilesAreSyncedCorrectlyWhenNoSnapshot(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W

nodeTypeId := internal.GetNodeTypeId(env)
nodeTypeId := testutil.GetCloud(t).NodeTypeID()
uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "basic", map[string]any{
"unique_id": uniqueId,
Expand Down
Loading

0 comments on commit 61b0c59

Please sign in to comment.