-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Consolidate and clean up tests / make them more uniform / refactor #743
Labels
Comments
alamb
added
enhancement
New feature or request
development-process
Related to development process of DataFusion
labels
Jul 18, 2021
This was referenced Jul 18, 2021
alamb
changed the title
Consolidate datafusion test / make them more uniform
Consolidate and clean up datafusion test / make them more uniform
Oct 15, 2021
alamb
changed the title
Consolidate and clean up datafusion test / make them more uniform
Consolidate and clean up tests / make them more uniform
Oct 15, 2021
This was referenced Nov 4, 2021
@alamb i believe we can mark task 2 completed now. If you agree then I can start looking at 3 |
Done, and THANK YOU! |
This was referenced Dec 15, 2021
alamb
changed the title
Consolidate and clean up tests / make them more uniform
Consolidate and clean up tests / make them more uniform / refactor
Dec 28, 2021
This was referenced Jan 26, 2022
This was referenced Feb 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
As @jorgecarleitao pointed out many moons ago, in apache/arrow#9936 (review) , the tests in datafusion/src/context.rs are not really unit tests. They are more like SQL integration tests.
There is also a small and languishing set of sql tests in
rust/datafusion/tests/sql.rs
that use a string comparison style that I personally find quite hard to upgrade.Among other things, this split makes it hard to understand the state of DataFusion's test coverage.
Since these tests are critical for DataFusion's quality, I propose a small reorganization so it is easier to find existing test coverage and write new ones:
Describe the solution you'd like
Specifically I propose:
assert_batches_eq!
macros to a non part of datafusion #745ExecutionContext
out of context.rs and into sql.rsrust/datafusion/src/test
to its own modulerust/test_helpers
(so that it can be shared with sql.rs)Then over time I imagine being able to organize the tests within sql.rs better (split into multiple modules, for example)
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
None
Additional context
Original mailing list thread: https://lists.apache.org/thread.html/r77cb6f55952803debdbe10bb615d00ec097942fb88c1a88496b3d69a%40%3Cdev.arrow.apache.org%3E
The text was updated successfully, but these errors were encountered: