Skip to content

Commit c18dfac

Browse files
pieternclaude
andauthored
direct: Use ignore_remote_changes in CRUD test assertions (#4441)
## Changes Introduce `testIgnoreFilter` helper to encapsulate field filtering logic based on `ignore_remote_changes` configuration from resource configuration files. Replace hardcoded filter for `updated_at` with a proper fix in the relevant resource. ## Tests This change adds an entry for a dashboard resource test to exercise this filter. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent b8586f5 commit c18dfac

File tree

2 files changed

+93
-17
lines changed

2 files changed

+93
-17
lines changed

bundle/direct/dresources/all_test.go

Lines changed: 87 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,26 @@ var testConfig map[string]any = map[string]any{
200200
OutputSchemaName: "main.myschema",
201201
},
202202
},
203+
204+
"dashboards": &resources.Dashboard{
205+
DashboardConfig: resources.DashboardConfig{
206+
DisplayName: "my-dashboard",
207+
ParentPath: "/Workspace/Users/user@example.com",
208+
WarehouseId: "test-warehouse-id",
209+
SerializedDashboard: map[string]any{
210+
"pages": []map[string]any{
211+
{
212+
"name": "page1",
213+
"displayName": "Page 1",
214+
"pageType": "PAGE_TYPE_CANVAS",
215+
},
216+
},
217+
},
218+
219+
DatasetCatalog: "main",
220+
DatasetSchema: "myschema",
221+
},
222+
},
203223
}
204224

205225
type prepareWorkspace func(client *databricks.WorkspaceClient) (any, error)
@@ -531,6 +551,60 @@ func TestAll(t *testing.T) {
531551
require.Len(t, m, len(SupportedResources))
532552
}
533553

554+
// testIgnoreFilter encapsulates the logic for filtering fields based on ignore_remote_changes config.
555+
type testIgnoreFilter struct {
556+
ignoreFields map[string]bool
557+
}
558+
559+
// newTestIgnoreFilter creates a filter from the adapter's resource configs.
560+
func newTestIgnoreFilter(adapter *Adapter) *testIgnoreFilter {
561+
ignoreFields := make(map[string]bool)
562+
for _, cfg := range []*ResourceLifecycleConfig{adapter.ResourceConfig(), adapter.GeneratedResourceConfig()} {
563+
if cfg == nil {
564+
continue
565+
}
566+
for _, p := range cfg.IgnoreRemoteChanges {
567+
ignoreFields[p.String()] = true
568+
}
569+
}
570+
return &testIgnoreFilter{ignoreFields: ignoreFields}
571+
}
572+
573+
// shouldIgnore returns true if the field at the given path should be ignored.
574+
func (f *testIgnoreFilter) shouldIgnore(path string) bool {
575+
if f.ignoreFields[path] {
576+
return true
577+
}
578+
// Check if this is a nested field under an ignored top-level field
579+
topLevelField := path
580+
if prefix, _, ok := strings.Cut(path, "."); ok {
581+
topLevelField = prefix
582+
}
583+
return f.ignoreFields[topLevelField]
584+
}
585+
586+
// filterChanges returns only the changes that should not be ignored.
587+
// It also filters out the "updated_at" timestamp field.
588+
func (f *testIgnoreFilter) filterChanges(changes []structdiff.Change) []structdiff.Change {
589+
var relevantChanges []structdiff.Change
590+
for _, change := range changes {
591+
fieldName := change.Path.String()
592+
if !f.shouldIgnore(fieldName) {
593+
relevantChanges = append(relevantChanges, change)
594+
}
595+
}
596+
return relevantChanges
597+
}
598+
599+
// requireEqual compares two structs and fails the test if there are differences
600+
// that are not in the ignore_remote_changes list.
601+
func (f *testIgnoreFilter) requireEqual(t *testing.T, expected, actual any, msgAndArgs ...any) {
602+
changes, err := structdiff.GetStructDiff(expected, actual, nil)
603+
require.NoError(t, err)
604+
relevantChanges := f.filterChanges(changes)
605+
require.Empty(t, relevantChanges, msgAndArgs...)
606+
}
607+
534608
func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.WorkspaceClient) {
535609
var inputConfig any
536610
var err error
@@ -576,10 +650,14 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
576650
require.NoError(t, err)
577651
require.NotNil(t, remappedState)
578652

653+
// Create filter for fields that should be ignored based on ignore_remote_changes config.
654+
ignoreFilter := newTestIgnoreFilter(adapter)
655+
579656
if remoteStateFromCreate != nil {
580657
remappedRemoteStateFromCreate, err := adapter.RemapState(remoteStateFromCreate)
581658
require.NoError(t, err)
582-
require.Equal(t, remappedState, remappedRemoteStateFromCreate)
659+
ignoreFilter.requireEqual(t, remappedState, remappedRemoteStateFromCreate,
660+
"unexpected differences between remappedState and remappedRemoteStateFromCreate")
583661
}
584662

585663
remoteStateFromWaitCreate, err := adapter.WaitAfterCreate(ctx, newState)
@@ -594,25 +672,17 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
594672
if remoteStateFromUpdate != nil {
595673
remappedStateFromUpdate, err := adapter.RemapState(remoteStateFromUpdate)
596674
require.NoError(t, err)
597-
changes, err := structdiff.GetStructDiff(remappedState, remappedStateFromUpdate, nil)
598-
require.NoError(t, err)
599-
// Filter out timestamp fields that are expected to differ in value
600-
var relevantChanges []structdiff.Change
601-
for _, change := range changes {
602-
fieldName := change.Path.String()
603-
if fieldName != "updated_at" {
604-
relevantChanges = append(relevantChanges, change)
605-
}
606-
}
607-
require.Empty(t, relevantChanges, "unexpected differences found: %v", relevantChanges)
675+
ignoreFilter.requireEqual(t, remappedState, remappedStateFromUpdate,
676+
"unexpected differences between remappedState and remappedStateFromUpdate")
608677
}
609678

610679
remoteStateFromWaitUpdate, err := adapter.WaitAfterUpdate(ctx, newState)
611680
require.NoError(t, err)
612681
if remoteStateFromWaitUpdate != nil {
613682
remappedStateFromWaitUpdate, err := adapter.RemapState(remoteStateFromWaitUpdate)
614683
require.NoError(t, err)
615-
require.Equal(t, remappedState, remappedStateFromWaitUpdate)
684+
ignoreFilter.requireEqual(t, remappedState, remappedStateFromWaitUpdate,
685+
"unexpected differences between remappedState and remappedStateFromWaitUpdate")
616686
}
617687
}
618688

@@ -631,6 +701,10 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
631701
// testserver can set field to backend-generated value
632702
return
633703
}
704+
// Skip fields configured in ignore_remote_changes.
705+
if ignoreFilter.shouldIgnore(path.String()) {
706+
return
707+
}
634708
// t.Logf("Testing %s v=%#v, remoteValue=%#v", path.String(), val, remoteValue)
635709
// We expect fields set explicitly to be preserved by testserver, which is true for all resources as of today.
636710
// If not true for your resource, add exception here:

bundle/direct/dresources/registered_model.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,15 @@ func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *
3434

3535
Aliases: model.Aliases,
3636
BrowseOnly: model.BrowseOnly,
37-
CreatedAt: model.CreatedAt,
38-
CreatedBy: model.CreatedBy,
3937
FullName: model.FullName,
4038
MetastoreId: model.MetastoreId,
4139
Owner: model.Owner,
42-
UpdatedAt: model.UpdatedAt,
43-
UpdatedBy: model.UpdatedBy,
40+
41+
// Clear output only fields. They should not show up on remote diff computation.
42+
CreatedAt: 0,
43+
CreatedBy: "",
44+
UpdatedAt: 0,
45+
UpdatedBy: "",
4446
}
4547
}
4648

0 commit comments

Comments
 (0)