From f665085c5aabf32509f65987b10f003a4c4cd21f Mon Sep 17 00:00:00 2001 From: Fred Lotter Date: Thu, 29 Aug 2024 10:03:15 +0200 Subject: [PATCH] simplify YAML field changes detection --- internals/plan/extensions_test.go | 4 ++-- internals/plan/plan.go | 19 ------------------- internals/plan/plan_test.go | 21 +++++++++++---------- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/internals/plan/extensions_test.go b/internals/plan/extensions_test.go index 8633af90..9885bf20 100644 --- a/internals/plan/extensions_test.go +++ b/internals/plan/extensions_test.go @@ -35,8 +35,8 @@ type inputLayer struct { } // PlanResult represents the final content of a combined plan. Since this -// package exclusively focuses extensions, all built-in sections are empty -// and ignored in the test results. +// test file exclusively focuses on extensions, all built-in sections are +// empty and ignored in the test results. type planResult struct { x *xSection y *ySection diff --git a/internals/plan/plan.go b/internals/plan/plan.go index 99e5dee2..3d8dc663 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -1251,11 +1251,6 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { "checks": &layer.Checks, "log-targets": &layer.LogTargets, } - // Make sure builtins contains the exact same fields as expected - // in the Layer type. - if !mapMatchKeys(builtins, builtinSections) { - panic("internal error: parsed fields and layer fields differ") - } sections := make(map[string]yaml.Node) // Deliberately pre-allocate at least an empty yaml.Node for every @@ -1340,20 +1335,6 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { return layer, err } -// mapMatchKeys returns true if the key list supplied is an exact match of the -// keys in the map (ordering is ignored). -func mapMatchKeys[M ~map[K]V, K comparable, V any](inMap M, keyList []K) bool { - if len(inMap) != len(keyList) { - return false - } - for key, _ := range inMap { - if !slices.Contains(keyList, key) { - return false - } - } - return true -} - func validServiceAction(action ServiceAction, additionalValid ...ServiceAction) bool { for _, v := range additionalValid { if action == v { diff --git a/internals/plan/plan_test.go b/internals/plan/plan_test.go index 4e4f22c2..99b617a8 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -20,7 +20,6 @@ import ( "os" "path/filepath" "reflect" - "slices" "strings" "time" "unicode" @@ -29,6 +28,7 @@ import ( "gopkg.in/yaml.v3" "github.com/canonical/pebble/internals/plan" + "github.com/canonical/pebble/internals/testutil" ) const ( @@ -2072,14 +2072,15 @@ func (s *S) TestStartStopOrderMultipleLanes(c *C) { c.Assert(lanes[2], DeepEquals, []string{"srv3"}) } -// TestLayerBuiltinCompatible ensures layerBuiltins used in the plan package -// reflects the same YAML fields as exposed in the Layer type. -func (s *S) TestLayerBuiltinCompatible(c *C) { - fields := structYamlFields(plan.Layer{}) - c.Assert(len(fields), Equals, len(plan.BuiltinSections)) - for _, field := range fields { - c.Assert(slices.Contains(plan.BuiltinSections, field), Equals, true) - } +// TestSectionFieldStability detects changes in plan.Layer and plan.Plan +// YAML fields, and fails on any change that could break hardcoded section +// fields in the code. On failure, please carefully inspect and update +// the plan library where required. +func (s *S) TestSectionFieldStability(c *C) { + layerFields := structYamlFields(plan.Layer{}) + c.Assert(layerFields, testutil.DeepUnsortedMatches, []string{"summary", "description", "services", "checks", "log-targets", "sections"}) + planFields := structYamlFields(plan.Plan{}) + c.Assert(planFields, testutil.DeepUnsortedMatches, []string{"services", "checks", "log-targets", "sections"}) } // structYamlFields extracts the YAML fields from a struct. If the YAML tag @@ -2090,7 +2091,7 @@ func structYamlFields(inStruct any) []string { for i := range inStructType.NumField() { fieldType := inStructType.Field(i) yamlTag := fieldType.Tag.Get("yaml") - if fieldType.IsExported() && yamlTag != "-" && !strings.Contains(yamlTag, ",inline") { + if fieldType.IsExported() && yamlTag != "-" { tag, _, _ := strings.Cut(fieldType.Tag.Get("yaml"), ",") if tag == "" { tag = firstLetterToLower(fieldType.Name)