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.

Sign up for GitHub

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

Jump to bottom

Merge enum-moduledef to pick up #217 #219

Merged
merged 4 commits into from
Aug 16, 2018
Merged

Merge enum-moduledef to pick up #217 #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c274f9
Fix bug with identity provenance identification.
robshakir Aug 10, 2018
b3694ac
Fix enumeration flake (#217)
robshakir Aug 16, 2018
07f285b
Merge branch 'master' into enum-moduledef
robshakir Aug 16, 2018
3ee5f6d
Merge branch 'enum-moduledef' of github.com:openconfig/ygot into enum…
robshakir Aug 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56,243 changes: 28,228 additions & 28,015 deletions exampleoc/oc.go
View file
Open in desktop

Large diffs are not rendered by default.

236 changes: 148 additions & 88 deletions ygen/codegen_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"io/ioutil"
"path/filepath"
"reflect"
"sort"
"testing"

"github.com/kylelemons/godebug/pretty"
Expand All @@ -33,6 +34,9 @@ const (
// TestRoot is the root of the test directory such that this is not
// repeated when referencing files.
TestRoot string = ""
// deflakeRuns specifies the number of runs of code generation that
// should be performed to check for flakes.
deflakeRuns int = 10
)

// TestFindMappableEntities tests the extraction of elements that are to be mapped
Expand Down Expand Up @@ -630,82 +634,91 @@ func TestSimpleStructs(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Set defaults within the supplied configuration for these tests.
if tt.inConfig.Caller == "" {
// Set the name of the caller explicitly to avoid issues when
// the unit tests are called by external test entities.
tt.inConfig.Caller = "codegen-tests"
}
tt.inConfig.StoreRawSchema = true
genCode := func() (*GeneratedGoCode, string, map[string]interface{}) {
// Set defaults within the supplied configuration for these tests.
if tt.inConfig.Caller == "" {
// Set the name of the caller explicitly to avoid issues when
// the unit tests are called by external test entities.
tt.inConfig.Caller = "codegen-tests"
}
tt.inConfig.StoreRawSchema = true

cg := NewYANGCodeGenerator(&tt.inConfig)
cg := NewYANGCodeGenerator(&tt.inConfig)

gotGeneratedCode, err := cg.GenerateGoCode(tt.inFiles, tt.inIncludePaths)
if err != nil && !tt.wantErr {
t.Errorf("%s: cg.GenerateCode(%v, %v): Config: %v, got unexpected error: %v, want: nil",
tt.name, tt.inFiles, tt.inIncludePaths, tt.inConfig, err)
return
}
gotGeneratedCode, err := cg.GenerateGoCode(tt.inFiles, tt.inIncludePaths)
if err != nil && !tt.wantErr {
t.Fatalf("%s: cg.GenerateCode(%v, %v): Config: %v, got unexpected error: %v, want: nil", tt.name, tt.inFiles, tt.inIncludePaths, tt.inConfig, err)
}

wantCode, rferr := ioutil.ReadFile(tt.wantStructsCodeFile)
if rferr != nil {
t.Errorf("%s: ioutil.ReadFile(%q) error: %v", tt.name, tt.wantStructsCodeFile, rferr)
return
}
// Write all the received structs into a single file such that
// it can be compared to the received file.
var gotCode bytes.Buffer
fmt.Fprint(&gotCode, gotGeneratedCode.CommonHeader)
fmt.Fprint(&gotCode, gotGeneratedCode.OneOffHeader)
for _, gotStruct := range gotGeneratedCode.Structs {
fmt.Fprint(&gotCode, gotStruct.String())
}

// Write all the received structs into a single file such that
// it can be compared to the received file.
var gotCode bytes.Buffer
fmt.Fprint(&gotCode, gotGeneratedCode.CommonHeader)
fmt.Fprint(&gotCode, gotGeneratedCode.OneOffHeader)
for _, gotStruct := range gotGeneratedCode.Structs {
fmt.Fprint(&gotCode, gotStruct.String())
}
for _, gotEnum := range gotGeneratedCode.Enums {
fmt.Fprint(&gotCode, gotEnum)
}

for _, gotEnum := range gotGeneratedCode.Enums {
fmt.Fprint(&gotCode, gotEnum)
}
// Write generated enumeration map out.
fmt.Fprint(&gotCode, gotGeneratedCode.EnumMap)

var gotJSON map[string]interface{}
if tt.inConfig.GenerateJSONSchema {
// Write the schema byte array out.
fmt.Fprint(&gotCode, gotGeneratedCode.JSONSchemaCode)
fmt.Fprint(&gotCode, gotGeneratedCode.EnumTypeMap)

// Write generated enumeration map out.
fmt.Fprint(&gotCode, gotGeneratedCode.EnumMap)
if err := json.Unmarshal(gotGeneratedCode.RawJSONSchema, &gotJSON); err != nil {
t.Fatalf("%s: json.Unmarshal(..., %v), could not unmarshal received JSON: %v", tt.name, gotGeneratedCode.RawJSONSchema, err)
}
}
return gotGeneratedCode, gotCode.String(), gotJSON
}

if tt.inConfig.GenerateJSONSchema {
// Write the schema byte array out.
fmt.Fprint(&gotCode, gotGeneratedCode.JSONSchemaCode)
fmt.Fprint(&gotCode, gotGeneratedCode.EnumTypeMap)
gotGeneratedCode, gotCode, gotJSON := genCode()

if tt.wantSchemaFile != "" {
wantSchema, rferr := ioutil.ReadFile(tt.wantSchemaFile)
if rferr != nil {
t.Errorf("%s: ioutil.ReadFile(%q) error: %v", tt.name, tt.wantSchemaFile, err)
return
}

var gotJSON map[string]interface{}
if err := json.Unmarshal(gotGeneratedCode.RawJSONSchema, &gotJSON); err != nil {
t.Errorf("%s: json.Unmarshal(..., %v), could not unmarshal received JSON: %v", tt.name, gotGeneratedCode.RawJSONSchema, err)
return
t.Fatalf("%s: ioutil.ReadFile(%q) error: %v", tt.name, tt.wantSchemaFile, rferr)
}

var wantJSON map[string]interface{}
if err := json.Unmarshal(wantSchema, &wantJSON); err != nil {
t.Errorf("%s: json.Unmarshal(..., [contents of %s]), could not unmarshal golden JSON file: %v", tt.name, tt.wantSchemaFile, err)
return
t.Fatalf("%s: json.Unmarshal(..., [contents of %s]), could not unmarshal golden JSON file: %v", tt.name, tt.wantSchemaFile, err)
}

if !reflect.DeepEqual(gotJSON, wantJSON) {
diff, _ := testutil.GenerateUnifiedDiff(string(gotGeneratedCode.RawJSONSchema), string(wantSchema))
t.Errorf("%s: GenerateGoCode(%v, %v), Config: %v, did not return correct JSON (file: %v), diff: \n%s", tt.name, tt.inFiles, tt.inIncludePaths, tt.inConfig, tt.wantSchemaFile, diff)
t.Fatalf("%s: GenerateGoCode(%v, %v), Config: %v, did not return correct JSON (file: %v), diff: \n%s", tt.name, tt.inFiles, tt.inIncludePaths, tt.inConfig, tt.wantSchemaFile, diff)
}
}

if gotCode.String() != string(wantCode) {
wantCode, rferr := ioutil.ReadFile(tt.wantStructsCodeFile)
if rferr != nil {
t.Fatalf("%s: ioutil.ReadFile(%q) error: %v", tt.name, tt.wantStructsCodeFile, rferr)
}

if gotCode != string(wantCode) {
// Use difflib to generate a unified diff between the
// two code snippets such that this is simpler to debug
// in the test output.
diff, _ := testutil.GenerateUnifiedDiff(gotCode.String(), string(wantCode))
diff, _ := testutil.GenerateUnifiedDiff(gotCode, string(wantCode))
t.Errorf("%s: GenerateGoCode(%v, %v), Config: %v, did not return correct code (file: %v), diff:\n%s",
tt.name, tt.inFiles, tt.inIncludePaths, tt.inConfig, tt.wantStructsCodeFile, diff)
}

for i := 0; i < deflakeRuns; i++ {
_, gotAttempt, _ := genCode()
if gotAttempt != gotCode {
diff, _ := testutil.GenerateUnifiedDiff(gotCode, gotAttempt)
t.Fatalf("flaky code generation, diff:\n%s", diff)
}
}
})
}
}
Expand Down Expand Up @@ -844,36 +857,38 @@ func TestFindRootEntries(t *testing.T) {
}}

for _, tt := range tests {
for compress, wantChildren := range map[bool][]string{true: tt.wantCompressRootChildren, false: tt.wantUncompressRootChildren} {
if err := createFakeRoot(tt.inStructs, tt.inRootElems, tt.inRootName, compress); err != nil {
t.Errorf("%s: cg.createFakeRoot(%v), CompressOCPaths: %v, got unexpected error: %v", tt.name, tt.inStructs, compress, err)
continue
}
t.Run(tt.name, func(t *testing.T) {
for compress, wantChildren := range map[bool][]string{true: tt.wantCompressRootChildren, false: tt.wantUncompressRootChildren} {
if err := createFakeRoot(tt.inStructs, tt.inRootElems, tt.inRootName, compress); err != nil {
t.Errorf("cg.createFakeRoot(%v), CompressOCPaths: %v, got unexpected error: %v", tt.inStructs, compress, err)
continue
}

rootElem, ok := tt.inStructs["/"]
if !ok {
t.Errorf("%s: cg.createFakeRoot(%v), CompressOCPaths: %v, could not find root element", tt.name, tt.inStructs, compress)
continue
}
rootElem, ok := tt.inStructs["/"]
if !ok {
t.Errorf("cg.createFakeRoot(%v), CompressOCPaths: %v, could not find root element", tt.inStructs, compress)
continue
}

gotChildren := map[string]bool{}
for n := range rootElem.Dir {
gotChildren[n] = true
}
gotChildren := map[string]bool{}
for n := range rootElem.Dir {
gotChildren[n] = true
}

for _, ch := range wantChildren {
if _, ok := rootElem.Dir[ch]; !ok {
t.Errorf("%s: cg.createFakeRoot(%v), CompressOCPaths: %v, could not find child %v in %v", tt.name, tt.inStructs, compress, ch, rootElem.Dir)
for _, ch := range wantChildren {
if _, ok := rootElem.Dir[ch]; !ok {
t.Errorf("cg.createFakeRoot(%v), CompressOCPaths: %v, could not find child %v in %v", tt.inStructs, compress, ch, rootElem.Dir)
}
gotChildren[ch] = false
}
gotChildren[ch] = false
}

for ch, ok := range gotChildren {
if ok == true {
t.Errorf("%s: cg.findRootentries(%v), CompressOCPaths: %v, did not expect child %v", tt.name, tt.inStructs, compress, ch)
for ch, ok := range gotChildren {
if ok == true {
t.Errorf("cg.findRootentries(%v), CompressOCPaths: %v, did not expect child %v", tt.inStructs, compress, ch)
}
}
}
}
})
}
}

Expand Down Expand Up @@ -1144,23 +1159,40 @@ func TestGenerateProto3(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.inConfig.Caller == "" {
// Override the caller if it is not set, to ensure that test
// output is deterministic.
tt.inConfig.Caller = "codegen-tests"
}

cg := NewYANGCodeGenerator(&tt.inConfig)
gotProto, err := cg.GenerateProto3(tt.inFiles, tt.inIncludePaths)
if (err != nil) != tt.wantErr {
t.Errorf("%s: cg.GenerateProto3(%v, %v), config: %v: got unexpected error: %v", tt.name, tt.inFiles, tt.inIncludePaths, tt.inConfig, err)
return
sortedPkgNames := func(pkgs map[string]string) []string {
wantPkgs := []string{}
for k := range tt.wantOutputFiles {
wantPkgs = append(wantPkgs, k)
}
sort.Strings(wantPkgs)
return wantPkgs
}

if tt.wantErr || err != nil {
return
genCode := func() *GeneratedProto3 {
if tt.inConfig.Caller == "" {
// Override the caller if it is not set, to ensure that test
// output is deterministic.
tt.inConfig.Caller = "codegen-tests"
}

cg := NewYANGCodeGenerator(&tt.inConfig)
gotProto, err := cg.GenerateProto3(tt.inFiles, tt.inIncludePaths)
if (err != nil) != tt.wantErr {
t.Fatalf("cg.GenerateProto3(%v, %v), config: %v: got unexpected error: %v", tt.inFiles, tt.inIncludePaths, tt.inConfig, err)
}

if tt.wantErr || err != nil {
return nil
}

return gotProto
}

gotProto := genCode()

allCode := bytes.Buffer{}

seenPkg := map[string]bool{}
for n := range gotProto.Packages {
seenPkg[n] = false
Expand All @@ -1174,7 +1206,9 @@ func TestGenerateProto3(t *testing.T) {
return a
}

for pkg, wantFile := range tt.wantOutputFiles {
wantPkgs := sortedPkgNames(tt.wantOutputFiles)
for _, pkg := range wantPkgs {
wantFile := tt.wantOutputFiles[pkg]
wantCode, err := ioutil.ReadFile(wantFile)
if err != nil {
t.Errorf("%s: ioutil.ReadFile(%v): could not read file for package %s", tt.name, wantFile, pkg)
Expand All @@ -1183,8 +1217,7 @@ func TestGenerateProto3(t *testing.T) {

gotPkg, ok := gotProto.Packages[pkg]
if !ok {
t.Errorf("%s: cg.GenerateProto3(%v, %v): did not find expected package %s in output, got: %#v, want key: %v", tt.name, tt.inFiles, tt.inIncludePaths, pkg, protoPkgs(gotProto.Packages), pkg)
return
t.Fatalf("%s: cg.GenerateProto3(%v, %v): did not find expected package %s in output, got: %#v, want key: %v", tt.name, tt.inFiles, tt.inIncludePaths, pkg, protoPkgs(gotProto.Packages), pkg)
}

// Mark this package as having been seen.
Expand All @@ -1203,11 +1236,13 @@ func TestGenerateProto3(t *testing.T) {
fmt.Fprintf(&gotCodeBuf, "%s", gotEnum)
}

allCode.WriteString(gotCodeBuf.String())

if diff := pretty.Compare(gotCodeBuf.String(), string(wantCode)); diff != "" {
if diffl, _ := testutil.GenerateUnifiedDiff(gotCodeBuf.String(), string(wantCode)); diffl != "" {
diff = diffl
}
t.Errorf("%s: cg.GenerateProto3(%v, %v) for package %s, did not get expected code (code file: %v), diff(-got,+want):\n%s", tt.name, tt.inFiles, tt.inIncludePaths, pkg, wantFile, diff)
t.Fatalf("%s: cg.GenerateProto3(%v, %v) for package %s, did not get expected code (code file: %v), diff(-got,+want):\n%s", tt.name, tt.inFiles, tt.inIncludePaths, pkg, wantFile, diff)
}
}

Expand All @@ -1216,6 +1251,31 @@ func TestGenerateProto3(t *testing.T) {
t.Errorf("%s: cg.GenerateProto3(%v, %v) did not test received package %v", tt.name, tt.inFiles, tt.inIncludePaths, pkg)
}
}

for i := 0; i < deflakeRuns; i++ {
got := genCode()
var gotCodeBuf bytes.Buffer

wantPkgs := sortedPkgNames(tt.wantOutputFiles)
for _, pkg := range wantPkgs {
gotPkg, ok := got.Packages[pkg]
if !ok {
t.Fatalf("%s: cg.GenerateProto3(%v, %v): did not find expected package %s in output, got: %#v, want key: %v", tt.name, tt.inFiles, tt.inIncludePaths, pkg, protoPkgs(gotProto.Packages), pkg)
}
fmt.Fprintf(&gotCodeBuf, gotPkg.Header)
for _, gotMsg := range gotPkg.Messages {
fmt.Fprintf(&gotCodeBuf, "%s\n", gotMsg)
}
for _, gotEnum := range gotPkg.Enums {
fmt.Fprintf(&gotCodeBuf, "%s", gotEnum)
}
}

if diff := pretty.Compare(gotCodeBuf.String(), allCode.String()); diff != "" {
diff, _ = testutil.GenerateUnifiedDiff(gotCodeBuf.String(), allCode.String())
t.Fatalf("flaky code generation iter: %d, diff(-got,+want):\n%s", i, diff)
}
}
})
}
}
Expand Down
19 changes: 16 additions & 3 deletions ygen/genstate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,20 @@ func (s *genState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscores b
identifierPath = fmt.Sprintf("%s/%s", identifierPath, identifierPathElem[i])
}

// For leaves that have an enumeration within a typedef that is within a union,
// we do not want to just use the place in the schema definition for de-duplication,
// since it becomes confusing for the user to have non-contextual names within
// this context. We therefore rewrite the identifier path to have the context
// that we are in. By default, we just use the name of the node, but in OpenConfig
// schemas we rely on the grandparent name.
if !isYANGBaseType(e.Type) {
idPfx := e.Name
if compressPaths && e.Parent != nil && e.Parent.Parent != nil {
idPfx = e.Parent.Parent.Name
}
identifierPath = fmt.Sprintf("%s%s", idPfx, identifierPath)
}

// If the leaf had already been encountered, then return the previously generated
// name, rather than generating a new name.
if definedName, ok := s.uniqueEnumeratedLeafNames[identifierPath]; ok {
Expand All @@ -458,8 +472,7 @@ func (s *genState) resolveEnumName(e *yang.Entry, compressPaths, noUnderscores b
// State or Config so would not be unique. The proposed name is
// handed to makeNameUnique to ensure that it does not clash with
// other defined names.
name := fmt.Sprintf("%s_%s_%s", yang.CamelCase(definingModName),
yang.CamelCase(e.Parent.Parent.Name), yang.CamelCase(e.Name))
name := fmt.Sprintf("%s_%s_%s", yang.CamelCase(definingModName), yang.CamelCase(e.Parent.Parent.Name), yang.CamelCase(e.Name))
if noUnderscores {
name = strings.Replace(name, "_", "", -1)
}
Expand Down Expand Up @@ -547,7 +560,7 @@ func (s *genState) enumeratedTypedefTypeName(args resolveTypeArgs, prefix string
// types which is defined in RFC6020/RFC7950) then we establish what the type
// that we must actually perform the mapping for is. By default, start with
// the type that is specified in the schema.
if _, builtin := yang.TypeKindFromName[args.yangType.Name]; !builtin {
if !isYANGBaseType(args.yangType) {
switch args.yangType.Kind {
case yang.Yenum, yang.Yidentityref:
// In the case of a typedef that specifies an enumeration or identityref
Expand Down
1 change: 1 addition & 0 deletions ygen/goelements.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ func (s *genState) goUnionSubTypes(subtype *yang.YangType, ctx *yang.Entry, curr
}
default:
var err error

mtype, err = s.yangTypeToGoType(resolveTypeArgs{yangType: subtype, contextEntry: ctx}, compressOCPaths)
if err != nil {
errs = append(errs, err)
Expand Down