-
Notifications
You must be signed in to change notification settings - Fork 177
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
Metadata integration tests #57
Conversation
@liurenjie1024, @Xuanwo , @Fokko it would be great if you could take a look. |
9698b27
to
a33934e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left a small suggestion. Thanks, @JanKaul !
260977d
to
f57c0ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great @JanKaul, I see a lot of good stuff in this PR, thanks for adding these tests 👍🏻
This PR adds integration tests for reading the table metadata from files. Some of the tests are designed to fail. With the current design of the serialization/deserialization the error doesn't specify which field is missing. So I couldn't do a precise check for certain tests.