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.

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

schema,tests,gen/go: more tests, gen union fixes. #257

Merged
merged 2 commits into from
Sep 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions datamodel/equal.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ package datamodel
// so an error should only happen if a Node implementation breaks that contract.
// It is generally not recommended to call DeepEqual on ADL nodes.
func DeepEqual(x, y Node) bool {
if x == nil || y == nil {
warpfork marked this conversation as resolved.
Show resolved Hide resolved
return x == y
}
xk, yk := x.Kind(), y.Kind()
if xk != yk {
return false
Expand Down
2 changes: 2 additions & 0 deletions datamodel/equal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ var deepEqualTests = []struct {
// Repeated basicnode.New invocations might return different pointers.
{"SameNodeDiffPointer", basic.NewString("same"), basic.NewString("same"), true},

{"NilVsNil", nil, nil, true},
{"NilVsNull", nil, datamodel.Null, false},
{"SameKindNull", datamodel.Null, datamodel.Null, true},
{"DiffKindNull", datamodel.Null, datamodel.Absent, false},
{"SameKindBool", basic.NewBool(true), basic.NewBool(true), true},
Expand Down
8 changes: 5 additions & 3 deletions node/bindnode/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,11 +754,13 @@ func (w *_structAssembler) AssembleValue() datamodel.NodeAssembler {
name := w.curKey.val.String()
field := w.schemaType.Field(name)
if field == nil {
panic(fmt.Sprintf("TODO: missing field %s in %s", name, w.schemaType.Name()))
// return nil, datamodel.ErrInvalidKey{
// TODO: should've been raised when the key was submitted (we have room to return errors there, but can only panic at this point in the game).
// TODO: should make well-typed errors for this.
panic(fmt.Sprintf("TODO: invalid key: %q is not a field in type %s", name, w.schemaType.Name()))
// panic(schema.ErrInvalidKey{
// TypeName: w.schemaType.Name(),
// Key: basicnode.NewString(name),
// }
// })
}
ftyp, ok := w.val.Type().FieldByName(fieldNameFromSchema(name))
if !ok {
Expand Down
3 changes: 3 additions & 0 deletions node/bindnode/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ func forSchemaTest(name string) []tests.EngineSubtest {
if name == "Links" {
return nil // TODO(mvdan): add support for links
}
if name == "UnionKeyedComplexChildren" {
return nil // Specifically, 'InhabitantB/repr-create_with_AK+AV' borks, because it needs representation-level AssignNode to support more.
}
return []tests.EngineSubtest{{
Engine: &bindEngine{},
}}
Expand Down
57 changes: 50 additions & 7 deletions node/tests/testcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,6 @@ func (tcase testcase) Test(t *testing.T, np, npr datamodel.NodePrototype) {
})
}

// Copy the node. This exercises the AssembleKey+AssembleValue path for maps, as opposed to the AssembleEntry path (which was exercised by the creation via unmarshal).
// This isn't especially informative for anything other than maps, but we do it universally anyway (it's not a significant time cost).
// TODO

// Copy the node, now at repr level. Again, this is for exercising AssembleKey+AssembleValue paths.
// TODO

// Serialize the type-level node, and check that we get the original json again.
// This exercises iterators on the type-level node.
// OR, if typeItr is present, do that instead (this is necessary when handling maps with complex keys or handling structs with absent values, since both of those are unserializable).
Expand Down Expand Up @@ -186,9 +179,59 @@ func (tcase testcase) Test(t *testing.T, np, npr datamodel.NodePrototype) {
testMarshal(t, n.(schema.TypedNode).Representation(), tcase.reprJson)
})
}

// Copy the node. If it's a map-like.
// This exercises the AssembleKey+AssembleValue path for maps (or things that act as maps, such as structs and unions),
// as opposed to the AssembleEntry path (which is what was exercised by the creation via unmarshal).
// Assumes that the iterators are working correctly.
if n.Kind() == datamodel.Kind_Map {
t.Run("type-create with AK+AV", func(t *testing.T) {
n3, err := shallowCopyMap(np, n)
Wish(t, err, ShouldEqual, nil)
Wish(t, datamodel.DeepEqual(n, n3), ShouldEqual, true)
})
}

// Copy the node, now at repr level. Again, this is for exercising AssembleKey+AssembleValue paths.
// Assumes that the iterators are working correctly.
if n.(schema.TypedNode).Representation().Kind() == datamodel.Kind_Map {
t.Run("repr-create with AK+AV", func(t *testing.T) {
n3, err := shallowCopyMap(npr, n.(schema.TypedNode).Representation())
Wish(t, err, ShouldEqual, nil)
Wish(t, datamodel.DeepEqual(n, n3), ShouldEqual, true)
})
}

})
}

func shallowCopyMap(np datamodel.NodePrototype, n datamodel.Node) (datamodel.Node, error) {
nb := np.NewBuilder()
ma, err := nb.BeginMap(n.Length())
if err != nil {
return nil, err
}
for itr := n.MapIterator(); !itr.Done(); {
k, v, err := itr.Next()
if err != nil {
return nil, err
}
if v.IsAbsent() {
continue
}
if err := ma.AssembleKey().AssignNode(k); err != nil {
return nil, err
}
if err := ma.AssembleValue().AssignNode(v); err != nil {
return nil, err
}
}
if err := ma.Finish(); err != nil {
return nil, err
}
return nb.Build(), nil
}

func testUnmarshal(t *testing.T, np datamodel.NodePrototype, data string, expectFail error) datamodel.Node {
t.Helper()
nb := np.NewBuilder()
Expand Down
2 changes: 1 addition & 1 deletion schema/gen/go/genUnion.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ func (g unionBuilderGenerator) emitMapAssemblerMethods(w io.Writer) {
ma.state = maState_midValue
switch ma.ca {
{{- range $i, $member := .Type.Members }}
case {{ $i }}:
case {{ add $i 1 }}:
{{- if (eq (dot.AdjCfg.UnionMemlayout dot.Type) "embedAll") }}
ma.ca{{ add $i 1 }}.w = &ma.w.x{{ add $i 1 }}
ma.ca{{ add $i 1 }}.m = &ma.cm
Expand Down
2 changes: 1 addition & 1 deletion schema/gen/go/genUnionReprKeyed.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func (g unionReprKeyedReprBuilderGenerator) emitMapAssemblerMethods(w io.Writer)
ma.state = maState_midValue
switch ma.ca {
{{- range $i, $member := .Type.Members }}
case {{ $i }}:
case {{ add $i 1 }}:
{{- if (eq (dot.AdjCfg.UnionMemlayout dot.Type) "embedAll") }}
ma.ca{{ add $i 1 }}.w = &ma.w.x{{ add $i 1 }}
ma.ca{{ add $i 1 }}.m = &ma.cm
Expand Down