You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/11/20 19:09:30 UTC
[PR] Use clap for sqllogicttest parsing, accept (and ignore) rust test harness arguments [arrow-datafusion]
alamb opened a new pull request, #8288:
URL: https://github.com/apache/arrow-datafusion/pull/8288
## Which issue does this PR close?
Closes https://github.com/apache/arrow-datafusion/issues/8287
Closes https://github.com/apache/arrow-datafusion/issues/4769
## Rationale for this change
Basically I want to be able to run sqllogictest in RustRover more conveniently (and also make it easier for @tustvold to work in DataFusion so he does it more ;) )
## What changes are included in this PR?
1. Rewrite the sqllogictest CLI parsing command to use `clap` https://github.com/apache/arrow-datafusion/issues/4769
2. Add support for the CLI options that RustRover seems to slap on to its invocation
## Are these changes tested?
Yes, by existing CI tests
## Are there any user-facing changes?
No, it is entirely developer facing
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use clap for sqllogicttest parsing, accept (and ignore) rust test harness arguments [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8288:
URL: https://github.com/apache/arrow-datafusion/pull/8288#issuecomment-1819724239
FYI @xudong963 (as you originally filed https://github.com/apache/arrow-datafusion/issues/4769) and @tustvold (as you originally mentioned https://github.com/apache/arrow-datafusion/issues/8287)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use clap for sqllogicttest parsing, accept (and ignore) rust test harness arguments [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8288:
URL: https://github.com/apache/arrow-datafusion/pull/8288#issuecomment-1821619370
> I worry this might be fragile if IDEs choose to enable other options.
I agree it is fragile (but easy to fix)
> I'm curious why you decided against making an explicit binary and then a test target that just calls the binary entrypoint but with no arguments, as was described in the initial ticket?
I tried this approach but then decided on this one because (at least for me in RustRover) I felt it would be easier to use / understand.
making a different binary ended up being something like `cargo run -p datafusion-sqllogictest --bin sqllogictest` or `cargo run -p datafusion-sqllogictest --bin runner` (though maybe my Cargo.toml fu is not as good as it could be)
And then when I started trying to add some documents on how to run this command, it got confusing quickly
Thus I figured it would be easier to understand if it just looked like the test binary.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Permit running `sqllogictest` as a rust test in IDEs (+ use clap for sqllogicttest parsing, accept (and ignore) rust test harness arguments) [arrow-datafusion]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #8288:
URL: https://github.com/apache/arrow-datafusion/pull/8288
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use clap for sqllogicttest parsing, accept (and ignore) rust test harness arguments [arrow-datafusion]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #8288:
URL: https://github.com/apache/arrow-datafusion/pull/8288#issuecomment-1819772954
I worry this might be fragile if IDEs choose to enable other options, I'm curious why you decided against making an explicit binary and then a test target that effectively calls the binary entrypoint but with no arguments?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Permit running `sqllogictest` as a rust test in IDEs (+ use clap for sqllogicttest parsing, accept (and ignore) rust test harness arguments) [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8288:
URL: https://github.com/apache/arrow-datafusion/pull/8288#issuecomment-1873328844
I added some additional warnings in 950611678 to reduce confusion
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org