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

Conversation

warpfork
Copy link
Collaborator

Increase the coverage produced by the schema test suites markedly, fixing a few old todos in the test system for typed nodes.

The additional exercise picks up some bugs in the codegen for unions.
(This was also reported by Adin, and I think possibly by Ian earlier,
though I only got around to reproducing it fully now.) Those bugs --
off-by-ones; uuf -- are now fixed.

Some todos were encountered in bindnode by the new coverage too.
Only one thing. But it's a bit more than I knew how to fix in one
sitting, so I've made that test skipped for now, hopefully to be
fixed soon.

Some error messages that confused me along the way are tweaked.

The datamodel.DeepEquals method is slightly less fragile on nils.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

datamodel/equal.go Show resolved Hide resolved
datamodel/equal.go Show resolved Hide resolved
…t suites markedly.

Fixes a few old todos in the test system for typed nodes.

The additional exercise picks up some bugs in the codegen for unions.
(This was also reported by Adin, and I think possibly by Ian earlier,
though I only got around to reproducing it fully now.)  Those bugs --
off-by-ones; uuf -- are now fixed.

Some todos were encountered in bindnode by the new coverage too.
Only one thing.  But it's a bit more than I knew how to fix in one
sitting, so I've made that test skipped for now, hopefully to be
fixed soon.

Some error messages that confused me along the way are tweaked.

The datamodel.DeepEquals method is slightly less fragile on nils.
@warpfork
Copy link
Collaborator Author

Added tests and made nils equal nils as suggested. Thanks for those. Will probably merge this shortly if not further objections; I think that addressed everything.

@mvdan
Copy link
Contributor

mvdan commented Sep 30, 2021

go ahead!

@warpfork warpfork merged commit 0b3aef3 into master Sep 30, 2021
@warpfork warpfork deleted the union-fixes-and-more-tests branch September 30, 2021 13:29
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants