Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extra tests for the sync block #1548

Merged
merged 2 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions bundle/tests/override_sync_test.go

This file was deleted.

19 changes: 19 additions & 0 deletions bundle/tests/sync/nil/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
bundle:
name: sync_nil

workspace:
host: https://acme.cloud.databricks.com/

sync:
include: ~
exclude: ~

targets:
development:

staging:
sync:
include:
- tests/*
exclude:
- dist
17 changes: 17 additions & 0 deletions bundle/tests/sync/nil_root/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
bundle:
name: sync_nil_root

workspace:
host: https://acme.cloud.databricks.com/

sync: ~

targets:
development:

staging:
sync:
include:
- tests/*
exclude:
- dist
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
bundle:
name: override_sync
name: sync_override

workspace:
host: https://acme.cloud.databricks.com/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
bundle:
name: override_sync
name: sync_override_no_root

workspace:
host: https://acme.cloud.databricks.com/
Expand Down
4 changes: 2 additions & 2 deletions bundle/tests/sync_include_exclude_no_matches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ import (
)

func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) {
b := loadTarget(t, "./override_sync", "development")
b := loadTarget(t, "./sync/override", "development")

diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), validate.ValidateSyncPatterns())
require.Len(t, diags, 3)
require.NoError(t, diags.Error())

require.Equal(t, diags[0].Severity, diag.Warning)
require.Equal(t, diags[0].Summary, "Pattern dist does not match any files")
require.Equal(t, diags[0].Location.File, filepath.Join("override_sync", "databricks.yml"))
require.Equal(t, diags[0].Location.File, filepath.Join("sync", "override", "databricks.yml"))
require.Equal(t, diags[0].Location.Line, 17)
require.Equal(t, diags[0].Location.Column, 11)
require.Equal(t, diags[0].Path.String(), "sync.exclude[0]")
Expand Down
65 changes: 65 additions & 0 deletions bundle/tests/sync_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package config_tests

import (
"path/filepath"
"testing"

"github.com/databricks/cli/bundle"
"github.com/stretchr/testify/assert"
)

func TestSyncOverride(t *testing.T) {
var b *bundle.Bundle

b = loadTarget(t, "./sync/override", "development")
assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)

b = loadTarget(t, "./sync/override", "staging")
assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)

b = loadTarget(t, "./sync/override", "prod")
assert.ElementsMatch(t, []string{filepath.FromSlash("src/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)
}

func TestSyncOverrideNoRootSync(t *testing.T) {
var b *bundle.Bundle

b = loadTarget(t, "./sync/override_no_root", "development")
assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)

b = loadTarget(t, "./sync/override_no_root", "staging")
assert.ElementsMatch(t, []string{filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)

b = loadTarget(t, "./sync/override_no_root", "prod")
assert.ElementsMatch(t, []string{}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)
}

func TestSyncNil(t *testing.T) {
var b *bundle.Bundle

b = loadTarget(t, "./sync/nil", "development")
assert.Nil(t, b.Config.Sync.Include)
assert.Nil(t, b.Config.Sync.Exclude)

b = loadTarget(t, "./sync/nil", "staging")
assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)
}

func TestSyncNilRoot(t *testing.T) {
var b *bundle.Bundle

b = loadTarget(t, "./sync/nil_root", "development")
assert.Nil(t, b.Config.Sync.Include)
assert.Nil(t, b.Config.Sync.Exclude)

b = loadTarget(t, "./sync/nil_root", "staging")
assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)
}
42 changes: 38 additions & 4 deletions libs/dyn/visit.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ import (
"slices"
)

// This error is returned if the path indicates that a map or sequence is expected, but the value is nil.
type cannotTraverseNilError struct {
p Path
}

func (e cannotTraverseNilError) Error() string {
component := e.p[len(e.p)-1]
switch {
case component.isKey():
return fmt.Sprintf("expected a map to index %q, found nil", e.p)
case component.isIndex():
return fmt.Sprintf("expected a sequence to index %q, found nil", e.p)
default:
panic("invalid component")
}
}

func IsCannotTraverseNilError(err error) bool {
var target cannotTraverseNilError
return errors.As(err, &target)
}

type noSuchKeyError struct {
p Path
}
Expand Down Expand Up @@ -70,11 +92,17 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts
switch {
case component.isKey():
// Expect a map to be set if this is a key.
m, ok := v.AsMap()
if !ok {
switch v.Kind() {
case KindMap:
// OK
case KindNil:
return InvalidValue, cannotTraverseNilError{path}
default:
return InvalidValue, fmt.Errorf("expected a map to index %q, found %s", path, v.Kind())
}

m := v.MustMap()

// Lookup current value in the map.
ev, ok := m.GetByString(component.key)
if !ok {
Expand Down Expand Up @@ -103,11 +131,17 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts

case component.isIndex():
// Expect a sequence to be set if this is an index.
s, ok := v.AsSequence()
if !ok {
switch v.Kind() {
case KindSequence:
// OK
case KindNil:
return InvalidValue, cannotTraverseNilError{path}
default:
return InvalidValue, fmt.Errorf("expected a sequence to index %q, found %s", path, v.Kind())
}

s := v.MustSequence()

// Lookup current value in the sequence.
if component.index < 0 || component.index >= len(s) {
return InvalidValue, indexOutOfBoundsError{path}
Expand Down
9 changes: 7 additions & 2 deletions libs/dyn/visit_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ type MapFunc func(Path, Value) (Value, error)

// Foreach returns a [MapFunc] that applies the specified [MapFunc] to each
// value in a map or sequence and returns the new map or sequence.
// If the input is nil, it returns nil.
func Foreach(fn MapFunc) MapFunc {
return func(p Path, v Value) (Value, error) {
switch v.Kind() {
case KindNil:
return v, nil
case KindMap:
m := v.MustMap().Clone()
for _, pair := range m.Pairs() {
Expand Down Expand Up @@ -75,8 +78,10 @@ func MapByPattern(v Value, p Pattern, fn MapFunc) (Value, error) {
return nv, nil
}

// Return original value if a key or index is missing.
if IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) {
// Return original value if:
// - any map or sequence is a nil, or
// - a key or index is missing
if IsCannotTraverseNilError(err) || IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) {
return v, nil
}

Expand Down
22 changes: 18 additions & 4 deletions libs/dyn/visit_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ func TestMapWithEmptyPath(t *testing.T) {
}

func TestMapOnNilValue(t *testing.T) {
var nv dyn.Value
var err error
_, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Key("foo")), nil)
assert.ErrorContains(t, err, `expected a map to index "foo", found nil`)
_, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Index(42)), nil)
assert.ErrorContains(t, err, `expected a sequence to index "[42]", found nil`)
nv, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Key("foo")), nil)
assert.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv)
nv, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Index(42)), nil)
assert.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv)
}

func TestMapFuncOnMap(t *testing.T) {
Expand Down Expand Up @@ -269,6 +272,17 @@ func TestMapForeachOnOtherError(t *testing.T) {
assert.ErrorContains(t, err, "expected a map or sequence, found int")
}

func TestMapForeachOnNil(t *testing.T) {
vin := dyn.NilValue

// Check that if foreach is applied to nil, it returns nil.
vout, err := dyn.Map(vin, ".", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, nil
}))
assert.NoError(t, err)
assert.Equal(t, dyn.NilValue, vout)
}

func TestMapByPatternOnNilValue(t *testing.T) {
var err error
_, err = dyn.MapByPattern(dyn.NilValue, dyn.NewPattern(dyn.AnyKey()), nil)
Expand Down
Loading