From 8960f9505d7800da3471588fb01e1cce1aee78c7 Mon Sep 17 00:00:00 2001 From: Thomas Rodgers Date: Wed, 17 Apr 2024 14:54:41 -0700 Subject: [PATCH] Exclude output only and parent fields from missing test detection (#10473) --- tools/diff-processor/detector/detector.go | 15 +++++++++++++++ tools/diff-processor/detector/detector_test.go | 17 +++++++++++++++-- tools/diff-processor/diff/diff.go | 4 +--- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/tools/diff-processor/detector/detector.go b/tools/diff-processor/detector/detector.go index 53ec0a9865fa..15b1b41fc5b5 100644 --- a/tools/diff-processor/detector/detector.go +++ b/tools/diff-processor/detector/detector.go @@ -7,6 +7,7 @@ import ( "github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff" "github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/reader" "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/zclconf/go-cty/cty" ) @@ -39,11 +40,25 @@ func DetectMissingTests(schemaDiff diff.SchemaDiff, allTests []*reader.Test) (ma return getMissingTestsForChanges(changedFields, allTests) } +// Convert SchemaDiff object to map of ResourceChanges objects. +// Also remove parent fields and output-only fields. func getChangedFieldsFromSchemaDiff(schemaDiff diff.SchemaDiff) map[string]ResourceChanges { changedFields := make(map[string]ResourceChanges) for resource, resourceDiff := range schemaDiff { resourceChanges := make(ResourceChanges) for field, fieldDiff := range resourceDiff.Fields { + if fieldDiff.New == nil { + // Skip deleted fields. + continue + } + if fieldDiff.New.Computed && !fieldDiff.New.Optional { + // Skip output-only fields. + continue + } + if _, ok := fieldDiff.New.Elem.(*schema.Resource); ok { + // Skip parent fields. + continue + } if fieldDiff.Old == nil { resourceChanges[field] = &Field{Added: true} } else { diff --git a/tools/diff-processor/detector/detector_test.go b/tools/diff-processor/detector/detector_test.go index a82f10d2ca69..f876c17fa051 100644 --- a/tools/diff-processor/detector/detector_test.go +++ b/tools/diff-processor/detector/detector_test.go @@ -20,11 +20,24 @@ func TestGetChangedFieldsFromSchemaDiff(t *testing.T) { schemaDiff: diff.SchemaDiff{ "covered_resource": diff.ResourceDiff{ Fields: map[string]diff.FieldDiff{ - "field_one": {}, + "field_one": { + New: &schema.Schema{}, + }, "field_two.field_three": { + New: &schema.Schema{}, Old: &schema.Schema{}, }, - "field_four.field_five.field_six": {}, + "field_four": { + New: &schema.Schema{ + Elem: &schema.Resource{}, + }, + }, + "field_four.field_five.field_six": { + New: &schema.Schema{}, + }, + "field_seven": { + New: &schema.Schema{Computed: true}, + }, }, }, }, diff --git a/tools/diff-processor/diff/diff.go b/tools/diff-processor/diff/diff.go index 486ac6de88a8..1f70760b1a89 100644 --- a/tools/diff-processor/diff/diff.go +++ b/tools/diff-processor/diff/diff.go @@ -9,9 +9,7 @@ import ( "golang.org/x/exp/maps" ) -// SchemaDiff is a nested map with field names as keys and Field objects -// as bottom-level values. -// Fields are assumed not to be covered until detected in a test. +// SchemaDiff is a nested map with resource names as top-level keys. type SchemaDiff map[string]ResourceDiff type ResourceDiff struct {