feat: Add dictionary support in integration test utility #342
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 87.96% 88.12% +0.15%
==========================================
Files 72 72
Lines 11733 11913 +180
==========================================
+ Hits 10321 10498 +177
- Misses 1412 1415 +3 ☔ View full report in Codecov by Sentry. |
paleolimbot
changed the title
|
I tried just running the new tests via archery and had a few more failures than I expected based on your disclaimer about known unsupported types. Specifically Full archery outputDoes the above look correct to you? And, as a side note, I ran into some trouble setting up archery correctly for the integration tests. I tried installing archery with just the |
|
Thank you for running the tests!
You're right that I didn't mention the stringview/binary view...nanoarrow doesn't have support for those, either yet. I don't see any output there I didn't expect.
I had no idea that the |
|
If there are no objections, I'll merge this tomorrow morning and continue with the rest of the integration test implementation! |
paleolimbot
force-pushed
the
more-integration
branch
from
fd25c39
to
49ce3d4
Compare
This was referenced Jan 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements dictionary support in the integration test utility and fixes a few problems identified with integration testing to ensure that it actually works end-to-end (via apache/arrow#39302 ). The changes are:
std::unordered_map<>with aUniqueSchemaand a few const overloads were missing)It can now be tested with
archery(after checking out apache/arrow#39302 ):The current failures are limited to the remaining unimplemented types (datetime types and decimal).
And for future me or anybody who has to/wants to launch a debugger with a segfaulting integration test in VSCode, it can be done with this launch.json: