Tidyup and testing of JSON Serialization Tests (#1710)

hack/generator/pkg/astmodel/test_case_injector.go

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT license.
+ */
+
+package astmodel
+
+// TestCaseInjector is a utility for injecting function definitions into resources and objects
+type TestCaseInjector struct {
+	// visitor is used to do the actual injection
+	visitor TypeVisitor
+}
+
+// NewTestCaseInjector creates a new function injector for modifying resources and objects
+func NewTestCaseInjector() *TestCaseInjector {
+	result := &TestCaseInjector{}
+
+	result.visitor = TypeVisitorBuilder{
+		VisitObjectType:   result.injectTestCaseIntoObject,
+		VisitResourceType: result.injectTestCaseIntoResource,
+	}.Build()
+
+	return result
+}
+
+// Inject modifies the passed type definition by injecting the passed function
+func (fi *TestCaseInjector) Inject(def TypeDefinition, cases ...TestCase) (TypeDefinition, error) {
+	result := def
+
+	for _, fn := range cases {
+		var err error
+		result, err = fi.visitor.VisitDefinition(result, fn)
+		if err != nil {
+			return TypeDefinition{}, err
+		}
+	}
+
+	return result, nil
+}
+
+// injectTestCaseIntoObject takes the function provided as a context and includes it on the
+// provided object type
+func (_ *TestCaseInjector) injectTestCaseIntoObject(
+	_ *TypeVisitor, ot *ObjectType, ctx interface{}) (Type, error) {
+	fn := ctx.(TestCase)
+	return ot.WithTestCase(fn), nil
+}
+
+// injectTestCaseIntoResource takes the function provided as a context and includes it on the
+// provided resource type
+func (_ *TestCaseInjector) injectTestCaseIntoResource(
+	_ *TypeVisitor, rt *ResourceType, ctx interface{}) (Type, error) {
+	fn := ctx.(TestCase)
+	return rt.WithTestCase(fn), nil
+}

hack/generator/pkg/codegen/pipeline/json_serialization_test_cases.go

@@ -8,72 +8,70 @@ package pipeline
 import (
 	"context"
 
+	"github.com/pkg/errors"
+
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/testcases"
 
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 )
 
+// InjectJsonSerializationTestsID is the unique identifier for this pipeline stage
+const InjectJsonSerializationTestsID = "injectJSONTestCases"
+
 func InjectJsonSerializationTests(idFactory astmodel.IdentifierFactory) Stage {
 
-	return MakeLegacyStage(
-		"jsonTestCases",
+	return MakeStage(
+		InjectJsonSerializationTestsID,
 		"Add test cases to verify JSON serialization",
-		func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
+		func(ctx context.Context, state *State) (*State, error) {
 			factory := makeObjectSerializationTestCaseFactory(idFactory)
-			result := make(astmodel.Types)
+			modifiedTypes := make(astmodel.Types)
 			var errs []error
-			for _, d := range types {
-				updated, err := factory.AddTestTo(d)
-				if err != nil {
-					errs = append(errs, err)
-				} else {
-					result[updated.Name()] = updated
+			for _, def := range state.Types() {
+				if factory.NeedsTest(def) {
+					updated, err := factory.AddTestTo(def)
+					if err != nil {
+						errs = append(errs, err)
+					} else {
+						modifiedTypes[updated.Name()] = updated
+					}
 				}
 			}
 
 			if len(errs) > 0 {
 				return nil, kerrors.NewAggregate(errs)
 			}
 
-			return result, nil
+			return state.WithTypes(state.Types().OverlayWith(modifiedTypes)), nil
 		})
 }
 
 type objectSerializationTestCaseFactory struct {
-	visitor   astmodel.TypeVisitor
+	injector  *astmodel.TestCaseInjector
 	idFactory astmodel.IdentifierFactory
 }
 
 func makeObjectSerializationTestCaseFactory(idFactory astmodel.IdentifierFactory) objectSerializationTestCaseFactory {
 	result := objectSerializationTestCaseFactory{
+		injector:  astmodel.NewTestCaseInjector(),
 		idFactory: idFactory,
 	}
 
-	result.visitor = astmodel.TypeVisitorBuilder{
-		VisitResourceType: result.injectTestCaseIntoResource,
-		VisitObjectType:   result.injectTestCaseIntoObject,
-	}.Build()
-
 	return result
 }
 
-func (s *objectSerializationTestCaseFactory) AddTestTo(def astmodel.TypeDefinition) (astmodel.TypeDefinition, error) {
-	return s.visitor.VisitDefinition(def, def.Name())
+func (s *objectSerializationTestCaseFactory) NeedsTest(def astmodel.TypeDefinition) bool {
+	_, ok := astmodel.AsPropertyContainer(def.Type())
+	return ok
 }
 
-func (s *objectSerializationTestCaseFactory) injectTestCaseIntoResource(
-	_ *astmodel.TypeVisitor, resource *astmodel.ResourceType, ctx interface{}) (astmodel.Type, error) {
-	name := ctx.(astmodel.TypeName)
-	testcase := testcases.NewJSONSerializationTestCase(name, resource, s.idFactory)
-	result := resource.WithTestCase(testcase)
-	return result, nil
-}
+func (s *objectSerializationTestCaseFactory) AddTestTo(def astmodel.TypeDefinition) (astmodel.TypeDefinition, error) {
+	container, ok := astmodel.AsPropertyContainer(def.Type())
+	if !ok {
+		return astmodel.TypeDefinition{}, errors.Errorf("expected %s to be a property container", def.Name())
+	}
 
-func (s *objectSerializationTestCaseFactory) injectTestCaseIntoObject(
-	_ *astmodel.TypeVisitor, objectType *astmodel.ObjectType, ctx interface{}) (astmodel.Type, error) {
-	name := ctx.(astmodel.TypeName)
-	testcase := testcases.NewJSONSerializationTestCase(name, objectType, s.idFactory)
-	result := objectType.WithTestCase(testcase)
-	return result, nil
+	testcase := testcases.NewJSONSerializationTestCase(def.Name(), container, s.idFactory)
+	return s.injector.Inject(def, testcase)
 }

hack/generator/pkg/codegen/testdata/TestNewARMCodeGeneratorFromConfigCreatesRightPipeline.golden

@@ -37,7 +37,7 @@ implementConvertibleSpecInterface         azure      Inject ConvertSpecTo() and
 implementConvertibleStatusInterface       azure      Inject ConvertStatusTo() and ConvertStatusFrom() to implement genruntime.ConvertibleStatus on each Status type
 injectOriginalGVKFunction                 azure      Inject the function OriginalGVK() into each Resource type
 simplifyDefinitions                                  Flatten definitions by removing wrapper types
-jsonTestCases                             azure      Add test cases to verify JSON serialization
+injectJSONTestCases                       azure      Add test cases to verify JSON serialization
 markStorageVersion                                   Mark the latest version of each resource as the storage version
 rogueCheck                                           Check for rogue definitions using AnyTypes
 ensureArmTypeExistsForEveryType           azure      Check that an ARM type exists for both Spec and Status of each resource

hack/generator/pkg/codegen/testdata/TestNewTestCodeGeneratorCreatesRightPipeline.golden

@@ -24,7 +24,7 @@ flattenProperties                                    Apply flattening to propert
 stripUnused                                          Strip unused types for test
 createConversionGraph                     azure      Create the graph of conversions between versions of each resource group
 simplifyDefinitions                                  Flatten definitions by removing wrapper types
-jsonTestCases                             azure      Add test cases to verify JSON serialization
+injectJSONTestCases                       azure      Add test cases to verify JSON serialization
 markStorageVersion                                   Mark the latest version of each resource as the storage version
 ensureArmTypeExistsForEveryType           azure      Check that an ARM type exists for both Spec and Status of each resource
 exportTestPackages                                   Export packages for test

hack/generator/pkg/test/asserts.go

@@ -17,7 +17,7 @@ import (
 
 // AssertFileGeneratesExpectedCode serialises the given FileDefinition as a golden file test, checking that the expected
 // results are generated
-func AssertFileGeneratesExpectedCode(t *testing.T, fileDef *astmodel.FileDefinition, testName string) {
+func AssertFileGeneratesExpectedCode(t *testing.T, fileDef astmodel.GoSourceFile, testName string) {
 	g := goldie.New(t)
 	err := g.WithTestNameForDir(true)
 	if err != nil {
@@ -34,8 +34,8 @@ func AssertFileGeneratesExpectedCode(t *testing.T, fileDef *astmodel.FileDefinit
 	g.Assert(t, testName, buf.Bytes())
 }
 
-// AssertPackagesGenerateExpectedCode creates a golden file for each package represented in the passed set of type
-// definitions, asserting that the generated content is expected
+// AssertPackagesGenerateExpectedCode creates a golden file for each package represented in the set of type definitions,
+// asserting that the generated content is expected
 func AssertPackagesGenerateExpectedCode(t *testing.T, types astmodel.Types) {
 	// Group type definitions by package
 	groups := make(map[astmodel.PackageReference][]astmodel.TypeDefinition)

hack/generator/pkg/test/file_definition.go

@@ -32,3 +32,25 @@ func CreateFileDefinition(definitions ...astmodel.TypeDefinition) *astmodel.File
 	fileDef := astmodel.NewFileDefinition(ref, definitions, packages)
 	return fileDef
 }
+
+func CreateTestFileDefinition(definitions ...astmodel.TypeDefinition) *astmodel.TestFileDefinition {
+	packages := make(map[astmodel.PackageReference]*astmodel.PackageDefinition)
+
+	// Use the package reference of the first definition for the whole file
+	ref, err := astmodel.PackageAsLocalPackage(definitions[0].Name().PackageReference)
+	if err != nil {
+		panic(errors.Wrap(err, "Expected first definition to have a local package reference - fix your test!"))
+	}
+
+	pkgDefinition := astmodel.NewPackageDefinition(ref.Group(), ref.PackageName(), ref.Version())
+	for _, def := range definitions {
+		pkgDefinition.AddDefinition(def)
+	}
+
+	packages[ref] = pkgDefinition
+
+	// put all definitions in one file, regardless.
+	// the package reference isn't really used here.
+	fileDef := astmodel.NewTestFileDefinition(ref, definitions, packages)
+	return fileDef
+}