Update tests for unions; several fixes #75
Conversation
The increased coverage found an infinite loop in iterators, so the fix for this is included in this commit.
Interesting to note that this causes the go-wish library to drop out of the imports list entirely.
This is probably now one of the nicer (directly-exercised) examples we have for how the type-level and representation-level views of data can differ with the use of schemas.
| @@ -35,36 +31,46 @@ func TestUnionKinded(t *testing.T) { | |||
| }), | |||
| )) | |||
|
|
|||
| // These are the same *type-level* as in TestUnionKeyedComplexChildren, | |||
| // but (of course) have very different representations. | |||
There was a problem hiding this comment.
I've been entertaining the thought of putting some methods on this testcase structure so that one can "derive" variations of them with only some fields changed, which could be useful for tests like this that poke at variations in representation.
Could be a future work thing.
| }, | ||
| } | ||
|
|
||
| test := func(t *testing.T, getPrototypeByName func(string) ipld.NodePrototype) { |
There was a problem hiding this comment.
These setup blocks (this and a dozen or so lines below) are getting somewhat redundant. It might soon be time to try to extract something here.
I've been wary to do this prematurely, and it'll be a bit tricky because reasons for doing distinct gen invocation are various (e.g. in these tests, it's always poking CfgUnionMemlayout; but that's not true in other tests which don't show up in this diff). But it's getting to the point it might be worth some looking into soon.
| } | ||
| } | ||
|
|
||
| t.Run("union-using-embed", func(t *testing.T) { |
There was a problem hiding this comment.
nitpicking some more, but these should be CamelCase for the sake of consistency. There are some examples in https://golang.org/pkg/testing/ too.
| @@ -162,3 +178,77 @@ func TestUnionKeyedComplexChildren(t *testing.T) { | |||
| }) | |||
| }) | |||
| } | |||
|
|
|||
| // TestUnionKeyedReset puts a union inside a list, so that we can use the list's reuse of assembler as a test of the assembler's reset feature. | |||
| // The value inside the union is also more complex than a scalar value so that we test resetting gets passed down, too. | |||
There was a problem hiding this comment.
so that we test that resetting...
I'm pretty sure, but not 100% sure :)
| typePoints: []testcasePoint{ | ||
| {"", ipld.ReprKind_Map}, | ||
| {"String", "whee"}, | ||
| //{"SmolStruct", ipld.ErrNotExists{}}, // TODO: need better error typing from traversal package. |
There was a problem hiding this comment.
nit: link to the issue in these TODOs? that way when the issue gets fixed, it's trivial to find the spots that can be improved
Update the tests targeting codegen'd unions to use the new test helper system introduced in #66 .
The increased coverage found an infinite loop in iterators, so the fix for this is included. Similarly, a bug in the assembler's reset mechanism was found, and is fixed.
With these two fixes, I'm also coincidentally able now able to use codegen'd unions in some pretty interesting larger demos without incident. More news on that shortly ;)