alamb commented on PR #6234:
URL:
https://github.com/apache/arrow-datafusion/pull/6234#issuecomment-1542080696
> Other than these this PR is LGTM!.
Thank you for the review @mustafasrepo -- I will make the changes
> However, I wonder whether we should keep an example test case each file.
Because, tests in .rs files are quite useful during development.
Can you explain how SQL tests in .rs files are more useful during
development than ones in .slt files? I suspect you are using a different
workflow from me (which is totally good and understandable, but I therefore
lack context).
When I am working on some new feature, I found it quite easy to make some
`new_feature.slt` file with just the SQL I want to run and then test with
`cargo test --test sqllogictests -- new_feature`. This is often better for me
than .rs tests because I can change the test and re-run it immediately without
having to wait for it to recompile.
> Novices can use these example test cases as starting point when they work
on new functionality. I am not really sure, whether it will be helpful. What
are you thoughts about it?
My thoughts on this topic are:
1. I agree that for people starting out with DataFusion development .rs
tests are easier (because they are familiar to all Rust developers).
sqllogictests are harder because they require learning another tool (though I
think the barrier is pretty low)
2. However, for SQL based tests, sqllogictest based tests are *much* easier
to maintain (specifically updating the tests with `cargo test --test
sqllogictests -- --complete`). In my opinion this long term benefit is worth
the short term cost
The reason I am spending effort porting (and asking others to help) port the
existing tests to sqllogictest is precisely to discourage new SQL tests written
in rust (so the code is easier to maintain long term)
🤔 if this makes sense to you, perhaps I can try to incorporate some of this
rational into the documentation to make it clearer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
