From d5df40ce3c1a4189d62216ab021d697bb580a2ba Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 26 Jan 2024 02:02:41 +0530 Subject: [PATCH 1/8] Add integration test for mlops-stacks initialization --- internal/init_test.go | 26 ++++++++++++++++++++++++++ libs/template/config.go | 5 ++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/internal/init_test.go b/internal/init_test.go index a2eda983c2..9555ec5abb 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -1,9 +1,13 @@ package internal import ( + "encoding/json" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { @@ -13,3 +17,25 @@ func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { _, _, err := RequireErrorRun(t, "bundle", "init", "./testdata/init/field-does-not-exist", "--output-dir", tmpDir) assert.EqualError(t, err, "failed to compute file content for bar.tmpl. variable \"does_not_exist\" not defined") } + +func TestAccBundleInitOnMlopsStacks(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + tmpDir1 := t.TempDir() + tmpDir2 := t.TempDir() + + // Create a config file with the project name and root dir + config := map[string]string{ + "input_project_name": "foobar", + "input_root_dir": "foobar", + } + b, err := json.Marshal(config) + require.NoError(t, err) + os.WriteFile(filepath.Join(tmpDir1, "config.json"), b, 0644) + + // Run bundle init + assert.NoFileExists(t, filepath.Join(tmpDir2, "foobar", "README.md")) + RequireSuccessfulRun(t, "bundle", "init", "/Users/shreyas.goenka/mlops-stack", "--output-dir", tmpDir2, "--config-file", filepath.Join(tmpDir1, "config.json")) + + // Assert that the README.md file was created + assert.FileExists(t, filepath.Join(tmpDir2, "foobar", "README.md")) +} diff --git a/libs/template/config.go b/libs/template/config.go index 5dd038e014..970e74ca95 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -89,7 +89,10 @@ func (c *config) assignValuesFromFile(path string) error { // Assigns default values from schema to input config map func (c *config) assignDefaultValues(r *renderer) error { - for name, property := range c.schema.Properties { + for _, p := range c.schema.OrderedProperties() { + name := p.Name + property := p.Schema + // Config already has a value assigned if _, ok := c.values[name]; ok { continue From 1ca69dd2c01cecfb62ff9666ab2dd7053515469c Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 26 Jan 2024 02:07:30 +0530 Subject: [PATCH 2/8] - --- internal/init_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/init_test.go b/internal/init_test.go index 9555ec5abb..fd99898599 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -34,7 +34,7 @@ func TestAccBundleInitOnMlopsStacks(t *testing.T) { // Run bundle init assert.NoFileExists(t, filepath.Join(tmpDir2, "foobar", "README.md")) - RequireSuccessfulRun(t, "bundle", "init", "/Users/shreyas.goenka/mlops-stack", "--output-dir", tmpDir2, "--config-file", filepath.Join(tmpDir1, "config.json")) + RequireSuccessfulRun(t, "bundle", "init", "mlops-stacks", "--output-dir", tmpDir2, "--config-file", filepath.Join(tmpDir1, "config.json")) // Assert that the README.md file was created assert.FileExists(t, filepath.Join(tmpDir2, "foobar", "README.md")) From 781fce89607674d727f098c99696495399024b16 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 1 Feb 2024 17:32:07 +0100 Subject: [PATCH 3/8] flesh out mlops-stacks test --- bundle/bundle_test.go | 5 +-- bundle/root_test.go | 34 +++++-------------- internal/init_test.go | 71 ++++++++++++++++++++++++++++++++++++---- internal/testutil/env.go | 23 +++++++++++++ 4 files changed, 98 insertions(+), 35 deletions(-) diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index 43477efd19..887a4ee83f 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/databricks/cli/bundle/env" + "github.com/databricks/cli/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -86,7 +87,7 @@ func TestBundleMustLoadFailureWithEnv(t *testing.T) { } func TestBundleMustLoadFailureIfNotFound(t *testing.T) { - chdir(t, t.TempDir()) + testutil.Chdir(t, t.TempDir()) _, err := MustLoad(context.Background()) require.Error(t, err, "unable to find bundle root") } @@ -105,7 +106,7 @@ func TestBundleTryLoadFailureWithEnv(t *testing.T) { } func TestBundleTryLoadOkIfNotFound(t *testing.T) { - chdir(t, t.TempDir()) + testutil.Chdir(t, t.TempDir()) b, err := TryLoad(context.Background()) assert.NoError(t, err) assert.Nil(t, b) diff --git a/bundle/root_test.go b/bundle/root_test.go index 88113546cb..d12a113791 100644 --- a/bundle/root_test.go +++ b/bundle/root_test.go @@ -8,29 +8,11 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/env" + "github.com/databricks/cli/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// Changes into specified directory for the duration of the test. -// Returns the current working directory. -func chdir(t *testing.T, dir string) string { - wd, err := os.Getwd() - require.NoError(t, err) - - abs, err := filepath.Abs(dir) - require.NoError(t, err) - - err = os.Chdir(abs) - require.NoError(t, err) - - t.Cleanup(func() { - err := os.Chdir(wd) - require.NoError(t, err) - }) - - return wd -} func TestRootFromEnv(t *testing.T) { ctx := context.Background() @@ -83,7 +65,7 @@ func TestRootLookup(t *testing.T) { t.Setenv(env.RootVariable, "") os.Unsetenv(env.RootVariable) - chdir(t, t.TempDir()) + testutil.Chdir(t, t.TempDir()) // Create databricks.yml file. f, err := os.Create(config.FileNames[0]) @@ -95,7 +77,7 @@ func TestRootLookup(t *testing.T) { require.NoError(t, err) // It should find the project root from $PWD. - wd := chdir(t, "./a/b/c") + wd := testutil.Chdir(t, "./a/b/c") root, err := mustGetRoot(ctx) require.NoError(t, err) require.Equal(t, wd, root) @@ -109,14 +91,14 @@ func TestRootLookupError(t *testing.T) { os.Unsetenv(env.RootVariable) // It can't find a project root from a temporary directory. - _ = chdir(t, t.TempDir()) + _ = testutil.Chdir(t, t.TempDir()) _, err := mustGetRoot(ctx) require.ErrorContains(t, err, "unable to locate bundle root") } func TestLoadYamlWhenIncludesEnvPresent(t *testing.T) { ctx := context.Background() - chdir(t, filepath.Join(".", "tests", "basic")) + testutil.Chdir(t, filepath.Join(".", "tests", "basic")) t.Setenv(env.IncludesVariable, "test") bundle, err := MustLoad(ctx) @@ -131,7 +113,7 @@ func TestLoadYamlWhenIncludesEnvPresent(t *testing.T) { func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { ctx := context.Background() dir := t.TempDir() - chdir(t, dir) + testutil.Chdir(t, dir) t.Setenv(env.RootVariable, dir) t.Setenv(env.IncludesVariable, "test") @@ -143,7 +125,7 @@ func TestLoadDefautlBundleWhenNoYamlAndRootAndIncludesEnvPresent(t *testing.T) { func TestErrorIfNoYamlNoRootEnvAndIncludesEnvPresent(t *testing.T) { ctx := context.Background() dir := t.TempDir() - chdir(t, dir) + testutil.Chdir(t, dir) t.Setenv(env.IncludesVariable, "test") _, err := MustLoad(ctx) @@ -153,7 +135,7 @@ func TestErrorIfNoYamlNoRootEnvAndIncludesEnvPresent(t *testing.T) { func TestErrorIfNoYamlNoIncludesEnvAndRootEnvPresent(t *testing.T) { ctx := context.Background() dir := t.TempDir() - chdir(t, dir) + testutil.Chdir(t, dir) t.Setenv(env.RootVariable, dir) _, err := MustLoad(ctx) diff --git a/internal/init_test.go b/internal/init_test.go index fd99898599..8dee93382e 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -1,11 +1,16 @@ package internal import ( + "context" "encoding/json" "os" "path/filepath" + "strconv" "testing" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/databricks-sdk-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,24 +23,76 @@ func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { assert.EqualError(t, err, "failed to compute file content for bar.tmpl. variable \"does_not_exist\" not defined") } +// This test tests the MLOps Stacks DAB e2e and thus there's a couple of special +// considerations to take note of: +// +// 1. Upstream changes to the MLOps Stacks DAB can cause this test to fail. +// In which case we should do one of: +// (a) Update this test to reflect the changes +// (b) Update the MLOps Stacks DAB to not break this test. Skip this test +// temporarily until the MLOps Stacks DAB is updated +// +// 2. While rare and to be avoided if possible, the CLI reserves the right to +// make changes that can break the MLOps Stacks DAB. In which case we should +// skip this test until the MLOps Stacks DAB is updated to work again. func TestAccBundleInitOnMlopsStacks(t *testing.T) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + env := GetEnvOrSkipTest(t, "CLOUD_ENV") + if env == "gcp" { + t.Skip("MLOps Stacks is not supported in GCP") + } tmpDir1 := t.TempDir() tmpDir2 := t.TempDir() + w, err := databricks.NewWorkspaceClient(&databricks.Config{}) + require.NoError(t, err) + // Create a config file with the project name and root dir - config := map[string]string{ - "input_project_name": "foobar", - "input_root_dir": "foobar", + initConfig := map[string]string{ + "input_project_name": "project_name", + "input_root_dir": "repo_name", + "input_include_models_in_unity_catalog": "no", + "input_cloud": env, } - b, err := json.Marshal(config) + b, err := json.Marshal(initConfig) require.NoError(t, err) os.WriteFile(filepath.Join(tmpDir1, "config.json"), b, 0644) // Run bundle init - assert.NoFileExists(t, filepath.Join(tmpDir2, "foobar", "README.md")) + assert.NoFileExists(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md")) RequireSuccessfulRun(t, "bundle", "init", "mlops-stacks", "--output-dir", tmpDir2, "--config-file", filepath.Join(tmpDir1, "config.json")) // Assert that the README.md file was created - assert.FileExists(t, filepath.Join(tmpDir2, "foobar", "README.md")) + assert.FileExists(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md")) + assertLocalFileContents(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md"), "This directory contains python code, notebooks and ML asset configs related to one ML project.") + assertLocalFileContents(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md"), "# project_name") + + // Validate the stack + testutil.Chdir(t, filepath.Join(tmpDir2, "repo_name", "project_name")) + RequireSuccessfulRun(t, "bundle", "validate") + + // Deploy the stack + RequireSuccessfulRun(t, "bundle", "deploy") + t.Cleanup(func() { + // Delete the stack + RequireSuccessfulRun(t, "bundle", "destroy", "--auto-approve") + }) + + // Get summary of the bundle deployment + stdout, _ := RequireSuccessfulRun(t, "bundle", "summary", "--output", "json") + summary := &config.Root{} + err = json.Unmarshal(stdout.Bytes(), summary) + require.NoError(t, err) + + // Assert resource Ids are not empty + assert.NotEmpty(t, summary.Resources.Experiments["experiment"].ID) + assert.NotEmpty(t, summary.Resources.Models["model"].ID) + assert.NotEmpty(t, summary.Resources.Jobs["batch_inference_job"].ID) + assert.NotEmpty(t, summary.Resources.Jobs["model_training_job"].ID) + + // Assert the batch inference job actually exists + batchJobId, err := strconv.ParseInt(summary.Resources.Jobs["batch_inference_job"].ID, 10, 64) + require.NoError(t, err) + job, err := w.Jobs.GetByJobId(context.Background(), batchJobId) + assert.NoError(t, err) + assert.Equal(t, "dev-project_name-batch-inference-job", job.Settings.Name) } diff --git a/internal/testutil/env.go b/internal/testutil/env.go index 39201c5b45..e1973ba82a 100644 --- a/internal/testutil/env.go +++ b/internal/testutil/env.go @@ -2,9 +2,12 @@ package testutil import ( "os" + "path/filepath" "runtime" "strings" "testing" + + "github.com/stretchr/testify/require" ) // CleanupEnvironment sets up a pristine environment containing only $PATH and $HOME. @@ -44,3 +47,23 @@ func GetEnvOrSkipTest(t *testing.T, name string) string { } return value } + +// Changes into specified directory for the duration of the test. +// Returns the current working directory. +func Chdir(t *testing.T, dir string) string { + wd, err := os.Getwd() + require.NoError(t, err) + + abs, err := filepath.Abs(dir) + require.NoError(t, err) + + err = os.Chdir(abs) + require.NoError(t, err) + + t.Cleanup(func() { + err := os.Chdir(wd) + require.NoError(t, err) + }) + + return wd +} From 274ae210c09c6f931804aaaea285250587589cc9 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 1 Feb 2024 17:41:41 +0100 Subject: [PATCH 4/8] lint --- internal/init_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/init_test.go b/internal/init_test.go index 8dee93382e..ff9b5ae03d 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -26,15 +26,15 @@ func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { // This test tests the MLOps Stacks DAB e2e and thus there's a couple of special // considerations to take note of: // -// 1. Upstream changes to the MLOps Stacks DAB can cause this test to fail. -// In which case we should do one of: -// (a) Update this test to reflect the changes -// (b) Update the MLOps Stacks DAB to not break this test. Skip this test -// temporarily until the MLOps Stacks DAB is updated +// 1. Upstream changes to the MLOps Stacks DAB can cause this test to fail. +// In which case we should do one of: +// (a) Update this test to reflect the changes +// (b) Update the MLOps Stacks DAB to not break this test. Skip this test +// temporarily until the MLOps Stacks DAB is updated // -// 2. While rare and to be avoided if possible, the CLI reserves the right to -// make changes that can break the MLOps Stacks DAB. In which case we should -// skip this test until the MLOps Stacks DAB is updated to work again. +// 2. While rare and to be avoided if possible, the CLI reserves the right to +// make changes that can break the MLOps Stacks DAB. In which case we should +// skip this test until the MLOps Stacks DAB is updated to work again. func TestAccBundleInitOnMlopsStacks(t *testing.T) { env := GetEnvOrSkipTest(t, "CLOUD_ENV") if env == "gcp" { From 451a8f5a3f79a191af037a9c13e77c9eea1d2775 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 1 Feb 2024 17:43:12 +0100 Subject: [PATCH 5/8] lint --- bundle/root_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/root_test.go b/bundle/root_test.go index d12a113791..e6c53e8249 100644 --- a/bundle/root_test.go +++ b/bundle/root_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/require" ) - func TestRootFromEnv(t *testing.T) { ctx := context.Background() dir := t.TempDir() From 9d2a726828828adaa880ab074f0ed3ba01d6becc Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 11 Mar 2024 15:04:49 +0100 Subject: [PATCH 6/8] parameterize the name of hte bundle/project --- internal/init_test.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/internal/init_test.go b/internal/init_test.go index d24b0cfb9b..93e3922d0b 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -3,6 +3,7 @@ package internal import ( "context" "encoding/json" + "fmt" "os" "path/filepath" "strconv" @@ -37,19 +38,21 @@ func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { // make changes that can break the MLOps Stacks DAB. In which case we should // skip this test until the MLOps Stacks DAB is updated to work again. func TestAccBundleInitOnMlopsStacks(t *testing.T) { - // env := GetEnvOrSkipTest(t, "CLOUD_ENV") - // if env == "gcp" { - // t.Skip("MLOps Stacks is not supported in GCP") - // } + env := GetEnvOrSkipTest(t, "CLOUD_ENV") + if env == "gcp" { + t.Skip("MLOps Stacks is not supported in GCP") + } tmpDir1 := t.TempDir() tmpDir2 := t.TempDir() w, err := databricks.NewWorkspaceClient(&databricks.Config{}) require.NoError(t, err) + projectName := RandomName("project_name_") + // Create a config file with the project name and root dir initConfig := map[string]string{ - "input_project_name": "project_name", + "input_project_name": projectName, "input_root_dir": "repo_name", "input_include_models_in_unity_catalog": "no", "input_cloud": env, @@ -59,16 +62,15 @@ func TestAccBundleInitOnMlopsStacks(t *testing.T) { os.WriteFile(filepath.Join(tmpDir1, "config.json"), b, 0644) // Run bundle init - assert.NoFileExists(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md")) + assert.NoFileExists(t, filepath.Join(tmpDir2, "repo_name", projectName, "README.md")) RequireSuccessfulRun(t, "bundle", "init", "mlops-stacks", "--output-dir", tmpDir2, "--config-file", filepath.Join(tmpDir1, "config.json")) // Assert that the README.md file was created - assert.FileExists(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md")) - assertLocalFileContents(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md"), "This directory contains python code, notebooks and ML asset configs related to one ML project.") - assertLocalFileContents(t, filepath.Join(tmpDir2, "repo_name", "project_name", "README.md"), "# project_name") + assert.FileExists(t, filepath.Join(tmpDir2, "repo_name", projectName, "README.md")) + assertLocalFileContents(t, filepath.Join(tmpDir2, "repo_name", projectName, "README.md"), fmt.Sprintf("# %s", projectName)) // Validate the stack - testutil.Chdir(t, filepath.Join(tmpDir2, "repo_name", "project_name")) + testutil.Chdir(t, filepath.Join(tmpDir2, "repo_name", projectName)) RequireSuccessfulRun(t, "bundle", "validate") // Deploy the stack @@ -95,7 +97,7 @@ func TestAccBundleInitOnMlopsStacks(t *testing.T) { require.NoError(t, err) job, err := w.Jobs.GetByJobId(context.Background(), batchJobId) assert.NoError(t, err) - assert.Equal(t, "dev-project_name-batch-inference-job", job.Settings.Name) + assert.Equal(t, fmt.Sprintf("dev-%s-batch-inference-job", projectName), job.Settings.Name) } func TestAccBundleInitHelpers(t *testing.T) { From 075f34e3f7ab3b01687b7788f7a7555e084b7dc7 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 11 Mar 2024 15:32:44 +0100 Subject: [PATCH 7/8] make test parallel --- internal/init_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/init_test.go b/internal/init_test.go index 93e3922d0b..a2cac2faa6 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -38,6 +38,7 @@ func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { // make changes that can break the MLOps Stacks DAB. In which case we should // skip this test until the MLOps Stacks DAB is updated to work again. func TestAccBundleInitOnMlopsStacks(t *testing.T) { + t.Parallel() env := GetEnvOrSkipTest(t, "CLOUD_ENV") if env == "gcp" { t.Skip("MLOps Stacks is not supported in GCP") From 49bf90318bd9cc878fc406ee68446f42724dba43 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 12 Mar 2024 15:06:34 +0100 Subject: [PATCH 8/8] enable on GCP --- internal/init_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/init_test.go b/internal/init_test.go index a2cac2faa6..45e6d031a3 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -40,9 +40,6 @@ func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { func TestAccBundleInitOnMlopsStacks(t *testing.T) { t.Parallel() env := GetEnvOrSkipTest(t, "CLOUD_ENV") - if env == "gcp" { - t.Skip("MLOps Stacks is not supported in GCP") - } tmpDir1 := t.TempDir() tmpDir2 := t.TempDir()