add unit tests

bundle/config/loader/process_include.go

@@ -3,12 +3,14 @@ package loader
 import (
 	"context"
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
+	"golang.org/x/exp/maps"
 )
 
 // Steps:
@@ -121,10 +123,21 @@ func validateSingleResourceDefined(r *config.Root, ext, typ string) diag.Diagnos
 	}
 
 	msg := strings.Builder{}
-	msg.WriteString(fmt.Sprintf("We recommend only defining a single %s when a file has the %s extension.", typ, ext))
+	msg.WriteString(fmt.Sprintf("We recommend only defining a single %s in a file with the %s extension.\n", typ, ext))
+
+	// Dedup the list of resources before adding them the diagnostic message. This
+	// is needed because we do not dedup earlier when gathering the resources and
+	// it's valid to define the same resource in both the resources and targets block.
 	msg.WriteString("The following resources are defined or configured in this file:\n")
+	setOfLines := map[string]struct{}{}
 	for _, r := range resources {
-		msg.WriteString(fmt.Sprintf("  - %s (%s)\n", r.key, r.typ))
+		setOfLines[fmt.Sprintf("  - %s (%s)\n", r.key, r.typ)] = struct{}{}
+	}
+	// Sort the line s to print to make the output deterministic.
+	listOfLines := maps.Keys(setOfLines)
+	sort.Strings(listOfLines)
+	for _, l := range listOfLines {
+		msg.WriteString(l)
 	}
 
 	locations := []dyn.Location{}
@@ -133,6 +146,13 @@ func validateSingleResourceDefined(r *config.Root, ext, typ string) diag.Diagnos
 		locations = append(locations, rr.value.Locations()...)
 		paths = append(paths, rr.path)
 	}
+	// Sort the locations and paths to make the output deterministic.
+	sort.Slice(locations, func(i, j int) bool {
+		return locations[i].String() < locations[j].String()
+	})
+	sort.Slice(paths, func(i, j int) bool {
+		return paths[i].String() < paths[j].String()
+	})
 
 	return diag.Diagnostics{
 		{

bundle/config/loader/process_include_test.go

@@ -10,13 +10,16 @@ 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"
 	"github.com/stretchr/testify/require"
 )
 
 func TestProcessInclude(t *testing.T) {
 	b := &bundle.Bundle{
-		RootPath: "testdata",
+		RootPath: "testdata/basic",
 		Config: config.Root{
 			Workspace: config.Workspace{
 				Host: "foo",
@@ -36,6 +39,38 @@ 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",
+			},
+		},
+	}
+
+	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.\nThe following resources are defined or configured in this file:\n  - bar (job)\n  - foo (job)\n",
+			Locations: []dyn.Location{
+				{File: "testdata/format/foo.job.yml", Line: 4, Column: 7},
+				{File: "testdata/format/foo.job.yml", Line: 7, Column: 7},
+			},
+			Paths: []dyn.Path{
+				dyn.MustPathFromString("resources.jobs.bar"),
+				dyn.MustPathFromString("resources.jobs.foo"),
+			},
+		},
+	}, diags)
+}
+
 func TestResourceNames(t *testing.T) {
 	names := []string{}
 	typ := reflect.TypeOf(config.Resources{})
@@ -72,125 +107,270 @@ func TestValidateFileFormat(t *testing.T) {
 			},
 		},
 	}
+	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.\nThe 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.\nThe 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.\nThe 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.\nThe 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.\nThe 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.\nThe 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, tc.fileName)
+			assert.Equal(t, tc.expected, diags)
+		})
+	}
 }
-
-// func TestGatherResources(t *testing.T) {
-// 	// TODO: Add location tests?
-// 	tcases := []struct {
-// 		name      string
-// 		resources config.Resources
-// 		targets   map[string]*config.Target
-// 		filenames map[string]string
-// 		expected  map[string]resource
-// 	}{
-// 		{
-// 			name:      "empty",
-// 			resources: config.Resources{},
-// 			expected:  map[string]resource{},
-// 		},
-// 		{
-// 			name: "one job",
-// 			resources: config.Resources{
-// 				Jobs: map[string]*resources.Job{
-// 					"job1": {},
-// 				},
-// 			},
-// 			// TODO CONTINUE: Setting file names for the resources defined here.
-// 			// and testing they are correctly aggregated.
-// 			expected: map[string]resource{
-// 				"job1": {
-// 					typ: "job",
-// 					p:   dyn.MustPathFromString("resources.jobs.job1"),
-// 				},
-// 			},
-// 		},
-// 		{
-// 			name: "three jobs",
-// 			resources: config.Resources{
-// 				Jobs: map[string]*resources.Job{
-// 					"job1": {},
-// 					"job2": {},
-// 					"job3": {},
-// 				},
-// 			},
-// 			expected: []resource{
-// 				{"job1", "job"},
-// 				{"job2", "job"},
-// 				{"job3", "job"},
-// 			},
-// 		},
-// 		{
-// 			name:      "only one experiment target",
-// 			resources: config.Resources{},
-// 			targets: map[string]*config.Target{
-// 				"target1": {
-// 					Resources: &config.Resources{
-// 						Experiments: map[string]*resources.MlflowExperiment{
-// 							"experiment1": {},
-// 						},
-// 					},
-// 				},
-// 			},
-// 			expected: []resource{
-// 				{"experiment1", "experiment"},
-// 			},
-// 		},
-// 		{
-// 			name: "multiple resources",
-// 			resources: config.Resources{
-// 				Jobs: map[string]*resources.Job{
-// 					"job1": {},
-// 				},
-// 				Pipelines: map[string]*resources.Pipeline{
-// 					"pipeline1": {},
-// 					"pipeline2": {},
-// 				},
-// 				Experiments: map[string]*resources.MlflowExperiment{
-// 					"experiment1": {},
-// 				},
-// 			},
-// 			targets: map[string]*config.Target{
-// 				"target1": {
-// 					Resources: &config.Resources{
-// 						ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{
-// 							"model_serving_endpoint1": {},
-// 						},
-// 					},
-// 				},
-// 				"target2": {
-// 					Resources: &config.Resources{
-// 						RegisteredModels: map[string]*resources.RegisteredModel{
-// 							"registered_model1": {},
-// 						},
-// 					},
-// 				},
-// 			},
-// 			expected: []resource{
-// 				{"job1", "job"},
-// 				{"pipeline1", "pipeline"},
-// 				{"pipeline2", "pipeline"},
-// 				{"experiment1", "experiment"},
-// 				{"model_serving_endpoint1", "model_serving_endpoint"},
-// 				{"registered_model1", "registered_model"},
-// 			},
-// 		},
-// 	}
-
-// 	for _, tc := range tcases {
-// 		t.Run(tc.name, func(t *testing.T) {
-// 			b := &bundle.Bundle{}
-
-// 			bundle.ApplyFunc(context.Background(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-// 				b.Config.Resources = tc.resources
-// 				b.Config.Targets = tc.targets
-// 				return nil
-// 			})
-
-// 			res, err := gatherResources(&b.Config)
-// 			require.NoError(t, err)
-
-// 			assert.Equal(t, tc.expected, res)
-// 		})
-// 	}
-
-// }

bundle/config/loader/testdata/format/databricks.yml

@@ -0,0 +1,2 @@
+bundle:
+  name: format_test

bundle/config/loader/testdata/format/foo.job.yml

@@ -0,0 +1,7 @@
+resources:
+  jobs:
+    foo:
+      name: foo
+
+    bar:
+      name: bar