From 4734249854d7eab987cb151b7a524d4fe429b764 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 26 Sep 2024 16:51:46 +0200 Subject: [PATCH] convert inline tests to files --- bundle/config/loader/process_include_test.go | 447 +++++------------- .../job_and_pipeline.experiment.yml | 11 + .../format_fail/job_and_pipeline.job.yml | 11 + .../format_fail/second_job_in_target.job.yml | 11 + .../format_fail/single_job.pipeline.yaml | 11 + .../testdata/format_fail/two_jobs.job.yml | 7 + .../format_fail/two_jobs_in_target.job.yml | 8 + .../testdata/format_pass/job_and_pipeline.yml | 11 + .../testdata/format_pass/one_job.job.yml | 11 + .../format_pass/one_pipeline.pipeline.yaml | 5 + .../loader/testdata/format_pass/two_job.yml | 7 + 11 files changed, 220 insertions(+), 320 deletions(-) create mode 100644 bundle/config/loader/testdata/format_fail/job_and_pipeline.experiment.yml create mode 100644 bundle/config/loader/testdata/format_fail/job_and_pipeline.job.yml create mode 100644 bundle/config/loader/testdata/format_fail/second_job_in_target.job.yml create mode 100644 bundle/config/loader/testdata/format_fail/single_job.pipeline.yaml create mode 100644 bundle/config/loader/testdata/format_fail/two_jobs.job.yml create mode 100644 bundle/config/loader/testdata/format_fail/two_jobs_in_target.job.yml create mode 100644 bundle/config/loader/testdata/format_pass/job_and_pipeline.yml create mode 100644 bundle/config/loader/testdata/format_pass/one_job.job.yml create mode 100644 bundle/config/loader/testdata/format_pass/one_pipeline.pipeline.yaml create mode 100644 bundle/config/loader/testdata/format_pass/two_job.yml diff --git a/bundle/config/loader/process_include_test.go b/bundle/config/loader/process_include_test.go index 664b669ae..1b90b19a0 100644 --- a/bundle/config/loader/process_include_test.go +++ b/bundle/config/loader/process_include_test.go @@ -9,8 +9,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/stretchr/testify/assert" @@ -39,37 +37,137 @@ func TestProcessInclude(t *testing.T) { assert.Equal(t, "bar", b.Config.Workspace.Host) } -func TestProcessIncludeValidatesFileFormat(t *testing.T) { - b := &bundle.Bundle{ - RootPath: "testdata/format", - Config: config.Root{ - Bundle: config.Bundle{ - Name: "format_test", - }, - }, +func TestProcessIncludeFormatPass(t *testing.T) { + for _, fileName := range []string{ + "one_job.job.yml", + "one_pipeline.pipeline.yaml", + "two_job.yml", + "job_and_pipeline.yml", + } { + t.Run(fileName, func(t *testing.T) { + b := &bundle.Bundle{ + RootPath: "testdata/format_pass", + Config: config.Root{ + Bundle: config.Bundle{ + Name: "format_test", + }, + }, + } + + m := ProcessInclude(filepath.Join(b.RootPath, fileName), fileName) + diags := bundle.Apply(context.Background(), b, m) + assert.Empty(t, diags) + }) } +} - m := ProcessInclude(filepath.Join(b.RootPath, "foo.job.yml"), "foo.job.yml") - diags := bundle.Apply(context.Background(), b, m) - require.NoError(t, diags.Error()) - - // Assert that the diagnostics contain the expected information - assert.Len(t, diags, 1) - assert.Equal(t, diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single job in a file with the .job.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - bar (job)\n - foo (job)\n", - Locations: []dyn.Location{ - {File: filepath.FromSlash("testdata/format/foo.job.yml"), Line: 4, Column: 7}, - {File: filepath.FromSlash("testdata/format/foo.job.yml"), Line: 7, Column: 7}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString("resources.jobs.bar"), - dyn.MustPathFromString("resources.jobs.foo"), +func TestProcessIncludeFormatFail(t *testing.T) { + for fileName, expectedDiags := range map[string]diag.Diagnostics{ + "single_job.pipeline.yaml": { + { + Severity: diag.Info, + Summary: "We recommend only defining a single pipeline in a file with the .pipeline.yaml extension.", + Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n", + Locations: []dyn.Location{ + {File: filepath.FromSlash("testdata/format_fail/single_job.pipeline.yaml"), Line: 11, Column: 11}, + {File: filepath.FromSlash("testdata/format_fail/single_job.pipeline.yaml"), Line: 4, Column: 7}, + }, + Paths: []dyn.Path{ + dyn.MustPathFromString("resources.jobs.job1"), + dyn.MustPathFromString("targets.target1.resources.jobs.job1"), + }, }, }, - }, diags) + "job_and_pipeline.job.yml": { + { + Severity: diag.Info, + Summary: "We recommend only defining a single job in a file with the .job.yml extension.", + Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - pipeline1 (pipeline)\n", + Locations: []dyn.Location{ + {File: filepath.FromSlash("testdata/format_fail/job_and_pipeline.job.yml"), Line: 12, Column: 11}, + {File: filepath.FromSlash("testdata/format_fail/job_and_pipeline.job.yml"), Line: 5, Column: 7}, + }, + Paths: []dyn.Path{ + dyn.MustPathFromString("resources.pipelines.pipeline1"), + dyn.MustPathFromString("targets.target1.resources.jobs.job1"), + }, + }, + }, + "job_and_pipeline.experiment.yml": { + { + Severity: diag.Info, + Summary: "We recommend only defining a single experiment in a file with the .experiment.yml extension.", + Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - pipeline1 (pipeline)\n", + Locations: []dyn.Location{ + {File: filepath.FromSlash("testdata/format_fail/job_and_pipeline.experiment.yml"), Line: 12, Column: 11}, + {File: filepath.FromSlash("testdata/format_fail/job_and_pipeline.experiment.yml"), Line: 5, Column: 7}, + }, + Paths: []dyn.Path{ + dyn.MustPathFromString("resources.pipelines.pipeline1"), + dyn.MustPathFromString("targets.target1.resources.jobs.job1"), + }, + }, + }, + "two_jobs.job.yml": { + { + Severity: diag.Info, + Summary: "We recommend only defining a single job in a file with the .job.yml extension.", + Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - job2 (job)\n", + Locations: []dyn.Location{ + {File: filepath.FromSlash("testdata/format_fail/two_jobs.job.yml"), Line: 5, Column: 7}, + {File: filepath.FromSlash("testdata/format_fail/two_jobs.job.yml"), Line: 8, Column: 7}, + }, + Paths: []dyn.Path{ + dyn.MustPathFromString("resources.jobs.job1"), + dyn.MustPathFromString("resources.jobs.job2"), + }, + }, + }, + "second_job_in_target.job.yml": { + { + Severity: diag.Info, + Summary: "We recommend only defining a single job in a file with the .job.yml extension.", + Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - job2 (job)\n", + Locations: []dyn.Location{ + {File: filepath.FromSlash("testdata/format_fail/second_job_in_target.job.yml"), Line: 12, Column: 11}, + {File: filepath.FromSlash("testdata/format_fail/second_job_in_target.job.yml"), Line: 5, Column: 7}, + }, + Paths: []dyn.Path{ + dyn.MustPathFromString("resources.jobs.job1"), + dyn.MustPathFromString("targets.target1.resources.jobs.job2"), + }, + }, + }, + "two_jobs_in_target.job.yml": { + { + Severity: diag.Info, + Summary: "We recommend only defining a single job in a file with the .job.yml extension.", + Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - job2 (job)\n", + Locations: []dyn.Location{ + {File: filepath.FromSlash("testdata/format_fail/two_jobs_in_target.job.yml"), Line: 6, Column: 11}, + {File: filepath.FromSlash("testdata/format_fail/two_jobs_in_target.job.yml"), Line: 8, Column: 11}, + }, + Paths: []dyn.Path{ + dyn.MustPathFromString("targets.target1.resources.jobs.job1"), + dyn.MustPathFromString("targets.target1.resources.jobs.job2"), + }, + }, + }, + } { + b := &bundle.Bundle{ + RootPath: "testdata/format_fail", + Config: config.Root{ + Bundle: config.Bundle{ + Name: "format_test", + }, + }, + } + + m := ProcessInclude(filepath.Join(b.RootPath, fileName), fileName) + diags := bundle.Apply(context.Background(), b, m) + require.Len(t, diags, 1) + assert.Equal(t, expectedDiags, diags) + } } func TestResourceNames(t *testing.T) { @@ -90,294 +188,3 @@ func TestResourceNames(t *testing.T) { assert.Contains(t, resourceTypes, name) } } - -func TestValidateFileFormat(t *testing.T) { - onlyJob := config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": {}, - }, - }, - Targets: map[string]*config.Target{ - "target1": { - Resources: &config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": {}, - }, - }, - }, - }, - } - onlyJobBundle := bundle.Bundle{Config: onlyJob} - - onlyPipeline := config.Root{ - Resources: config.Resources{ - Pipelines: map[string]*resources.Pipeline{ - "pipeline1": {}, - }, - }, - } - onlyPipelineBundle := bundle.Bundle{Config: onlyPipeline} - - bothJobAndPipeline := config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": {}, - }, - }, - Targets: map[string]*config.Target{ - "target1": { - Resources: &config.Resources{ - Pipelines: map[string]*resources.Pipeline{ - "pipeline1": {}, - }, - }, - }, - }, - } - bothJobAndPipelineBundle := bundle.Bundle{Config: bothJobAndPipeline} - - twoJobs := config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": {}, - "job2": {}, - }, - }, - } - twoJobsBundle := bundle.Bundle{Config: twoJobs} - - twoJobsTopLevelAndTarget := config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": {}, - }, - }, - Targets: map[string]*config.Target{ - "target1": { - Resources: &config.Resources{ - Jobs: map[string]*resources.Job{ - "job2": {}, - }, - }, - }, - }, - } - twoJobsTopLevelAndTargetBundle := bundle.Bundle{Config: twoJobsTopLevelAndTarget} - - twoJobsInTarget := config.Root{ - Targets: map[string]*config.Target{ - "target1": { - Resources: &config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": {}, - "job2": {}, - }, - }, - }, - }, - } - twoJobsInTargetBundle := bundle.Bundle{Config: twoJobsInTarget} - - tcases := []struct { - name string - bundle *bundle.Bundle - expected diag.Diagnostics - fileName string - locations map[string]dyn.Location - }{ - { - name: "single job", - bundle: &onlyJobBundle, - expected: nil, - fileName: "foo.job.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.job.yml", Line: 1, Column: 1}, - }, - }, - { - name: "single pipeline", - bundle: &onlyPipelineBundle, - expected: nil, - fileName: "foo.pipeline.yml", - locations: map[string]dyn.Location{ - "resources.pipelines.pipeline1": {File: "foo.pipeline.yaml", Line: 1, Column: 1}, - }, - }, - { - name: "single job but extension is pipeline", - bundle: &onlyJobBundle, - expected: diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single pipeline in a file with the .pipeline.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n", - Locations: []dyn.Location{ - {File: "foo.pipeline.yml", Line: 1, Column: 1}, - {File: "foo.pipeline.yml", Line: 2, Column: 2}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString("resources.jobs.job1"), - dyn.MustPathFromString("targets.target1.resources.jobs.job1"), - }, - }, - }, - fileName: "foo.pipeline.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.pipeline.yml", Line: 1, Column: 1}, - "targets.target1.resources.jobs.job1": {File: "foo.pipeline.yml", Line: 2, Column: 2}, - }, - }, - { - name: "job and pipeline", - bundle: &bothJobAndPipelineBundle, - expected: nil, - fileName: "foo.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.yml", Line: 1, Column: 1}, - "targets.target1.resources.pipelines.pipeline1": {File: "foo.yml", Line: 2, Column: 2}, - }, - }, - { - name: "job and pipeline but extension is job", - bundle: &bothJobAndPipelineBundle, - expected: diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single job in a file with the .job.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - pipeline1 (pipeline)\n", - Locations: []dyn.Location{ - {File: "foo.job.yml", Line: 1, Column: 1}, - {File: "foo.job.yml", Line: 2, Column: 2}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString("resources.jobs.job1"), - dyn.MustPathFromString("targets.target1.resources.pipelines.pipeline1"), - }, - }, - }, - fileName: "foo.job.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.job.yml", Line: 1, Column: 1}, - "targets.target1.resources.pipelines.pipeline1": {File: "foo.job.yml", Line: 2, Column: 2}, - }, - }, - { - name: "job and pipeline but extension is experiment", - bundle: &bothJobAndPipelineBundle, - expected: diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single experiment in a file with the .experiment.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - pipeline1 (pipeline)\n", - Locations: []dyn.Location{ - {File: "foo.experiment.yml", Line: 1, Column: 1}, - {File: "foo.experiment.yml", Line: 2, Column: 2}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString("resources.jobs.job1"), - dyn.MustPathFromString("targets.target1.resources.pipelines.pipeline1"), - }, - }, - }, - fileName: "foo.experiment.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.experiment.yml", Line: 1, Column: 1}, - "targets.target1.resources.pipelines.pipeline1": {File: "foo.experiment.yml", Line: 2, Column: 2}, - }, - }, - { - name: "two jobs", - bundle: &twoJobsBundle, - expected: diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single job in a file with the .job.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - job2 (job)\n", - Locations: []dyn.Location{ - {File: "foo.job.yml", Line: 1, Column: 1}, - {File: "foo.job.yml", Line: 2, Column: 2}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString("resources.jobs.job1"), - dyn.MustPathFromString("resources.jobs.job2"), - }, - }, - }, - fileName: "foo.job.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.job.yml", Line: 1, Column: 1}, - "resources.jobs.job2": {File: "foo.job.yml", Line: 2, Column: 2}, - }, - }, - { - name: "two jobs but extension is simple yaml", - bundle: &twoJobsBundle, - expected: nil, - fileName: "foo.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.yml", Line: 1, Column: 1}, - "resources.jobs.job2": {File: "foo.yml", Line: 2, Column: 2}, - }, - }, - { - name: "two jobs in top level and target", - bundle: &twoJobsTopLevelAndTargetBundle, - expected: diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single job in a file with the .job.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - job2 (job)\n", - Locations: []dyn.Location{ - {File: "foo.job.yml", Line: 1, Column: 1}, - {File: "foo.job.yml", Line: 2, Column: 2}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString("resources.jobs.job1"), - dyn.MustPathFromString("targets.target1.resources.jobs.job2"), - }, - }, - }, - fileName: "foo.job.yml", - locations: map[string]dyn.Location{ - "resources.jobs.job1": {File: "foo.job.yml", Line: 1, Column: 1}, - "targets.target1.resources.jobs.job2": {File: "foo.job.yml", Line: 2, Column: 2}, - }, - }, - { - name: "two jobs in target", - bundle: &twoJobsInTargetBundle, - expected: diag.Diagnostics{ - { - Severity: diag.Info, - Summary: "We recommend only defining a single job in a file with the .job.yml extension.", - Detail: "The following resources are defined or configured in this file:\n - job1 (job)\n - job2 (job)\n", - Locations: []dyn.Location{ - {File: "foo.job.yml", Line: 1, Column: 1}, - {File: "foo.job.yml", Line: 2, Column: 2}, - }, - Paths: []dyn.Path{ - dyn.MustPathFromString(("targets.target1.resources.jobs.job1")), - dyn.MustPathFromString("targets.target1.resources.jobs.job2"), - }, - }, - }, - fileName: "foo.job.yml", - locations: map[string]dyn.Location{ - "targets.target1.resources.jobs.job1": {File: "foo.job.yml", Line: 1, Column: 1}, - "targets.target1.resources.jobs.job2": {File: "foo.job.yml", Line: 2, Column: 2}, - }, - }, - } - - for _, tc := range tcases { - t.Run(tc.name, func(t *testing.T) { - for k, v := range tc.locations { - bundletest.SetLocation(tc.bundle, k, []dyn.Location{v}) - } - - diags := validateFileFormat(tc.bundle.Config.Value(), tc.fileName) - assert.Equal(t, tc.expected, diags) - }) - } -} diff --git a/bundle/config/loader/testdata/format_fail/job_and_pipeline.experiment.yml b/bundle/config/loader/testdata/format_fail/job_and_pipeline.experiment.yml new file mode 100644 index 000000000..0867fcae4 --- /dev/null +++ b/bundle/config/loader/testdata/format_fail/job_and_pipeline.experiment.yml @@ -0,0 +1,11 @@ +resources: + pipelines: + pipeline1: + name: pipeline1 + +targets: + target1: + resources: + jobs: + job1: + name: job1 diff --git a/bundle/config/loader/testdata/format_fail/job_and_pipeline.job.yml b/bundle/config/loader/testdata/format_fail/job_and_pipeline.job.yml new file mode 100644 index 000000000..0867fcae4 --- /dev/null +++ b/bundle/config/loader/testdata/format_fail/job_and_pipeline.job.yml @@ -0,0 +1,11 @@ +resources: + pipelines: + pipeline1: + name: pipeline1 + +targets: + target1: + resources: + jobs: + job1: + name: job1 diff --git a/bundle/config/loader/testdata/format_fail/second_job_in_target.job.yml b/bundle/config/loader/testdata/format_fail/second_job_in_target.job.yml new file mode 100644 index 000000000..628b9879f --- /dev/null +++ b/bundle/config/loader/testdata/format_fail/second_job_in_target.job.yml @@ -0,0 +1,11 @@ +resources: + jobs: + job1: + name: job1 + +targets: + target1: + resources: + jobs: + job2: + name: job2 diff --git a/bundle/config/loader/testdata/format_fail/single_job.pipeline.yaml b/bundle/config/loader/testdata/format_fail/single_job.pipeline.yaml new file mode 100644 index 000000000..91af87cdc --- /dev/null +++ b/bundle/config/loader/testdata/format_fail/single_job.pipeline.yaml @@ -0,0 +1,11 @@ +resources: + jobs: + job1: + name: job1 + +targets: + target1: + resources: + jobs: + job1: + description: job1 diff --git a/bundle/config/loader/testdata/format_fail/two_jobs.job.yml b/bundle/config/loader/testdata/format_fail/two_jobs.job.yml new file mode 100644 index 000000000..81ff90a75 --- /dev/null +++ b/bundle/config/loader/testdata/format_fail/two_jobs.job.yml @@ -0,0 +1,7 @@ +resources: + jobs: + job1: + name: job1 + + job2: + name: job2 diff --git a/bundle/config/loader/testdata/format_fail/two_jobs_in_target.job.yml b/bundle/config/loader/testdata/format_fail/two_jobs_in_target.job.yml new file mode 100644 index 000000000..3b489c1f7 --- /dev/null +++ b/bundle/config/loader/testdata/format_fail/two_jobs_in_target.job.yml @@ -0,0 +1,8 @@ +targets: + target1: + resources: + jobs: + job1: + description: job1 + job2: + description: job2 diff --git a/bundle/config/loader/testdata/format_pass/job_and_pipeline.yml b/bundle/config/loader/testdata/format_pass/job_and_pipeline.yml new file mode 100644 index 000000000..0867fcae4 --- /dev/null +++ b/bundle/config/loader/testdata/format_pass/job_and_pipeline.yml @@ -0,0 +1,11 @@ +resources: + pipelines: + pipeline1: + name: pipeline1 + +targets: + target1: + resources: + jobs: + job1: + name: job1 diff --git a/bundle/config/loader/testdata/format_pass/one_job.job.yml b/bundle/config/loader/testdata/format_pass/one_job.job.yml new file mode 100644 index 000000000..91af87cdc --- /dev/null +++ b/bundle/config/loader/testdata/format_pass/one_job.job.yml @@ -0,0 +1,11 @@ +resources: + jobs: + job1: + name: job1 + +targets: + target1: + resources: + jobs: + job1: + description: job1 diff --git a/bundle/config/loader/testdata/format_pass/one_pipeline.pipeline.yaml b/bundle/config/loader/testdata/format_pass/one_pipeline.pipeline.yaml new file mode 100644 index 000000000..6b16641e9 --- /dev/null +++ b/bundle/config/loader/testdata/format_pass/one_pipeline.pipeline.yaml @@ -0,0 +1,5 @@ +# TODO: Remove all the schema inlined references +resources: + pipelines: + pipeline1: + name: pipeline1 diff --git a/bundle/config/loader/testdata/format_pass/two_job.yml b/bundle/config/loader/testdata/format_pass/two_job.yml new file mode 100644 index 000000000..81ff90a75 --- /dev/null +++ b/bundle/config/loader/testdata/format_pass/two_job.yml @@ -0,0 +1,7 @@ +resources: + jobs: + job1: + name: job1 + + job2: + name: job2