-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add --complete
auto completion mode to sqllogictests
#4665
Add --complete
auto completion mode to sqllogictests
#4665
Conversation
info!("Using complete mode to complete {}", path.display()); | ||
let col_separator = " "; | ||
let validator = default_validator; | ||
update_test_file(path, runner, col_separator, validator) |
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 calls the excellent function update_test_file
from @xxchan added in risinglightdb/sqllogictest-rs#130 👨🍳 👌
|
||
|
||
|
||
# array_agg_zero |
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.
Writing these tests was amazing -- I just added the queries and the --complete
mode added the expected results ❤️
I am quite pleased that this worked so well |
I'll review it today, love the feature!!! |
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.
Except for a few minor flaws, others lgtm, thanks @alamb
Currently, we only have one dir test_files
which contains all test files. As more and more tests become available, we can categorize them by adding some new subdirs under test_files.
Btw, maybe we can use clap
and WalkDir
in the future to replace Options
struct.
Co-authored-by: xudong.w <[email protected]>
Good idea -- filed #4709
I agree using clap in the future would be a good idea! |
Benchmark runs are scheduled for baseline = c9d6118 and contender = 04d095d. 04d095d is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #4570
re #4460
Rationale for this change
TLDR I want to be able to write a script and capture expected output easily
See #4570
What changes are included in this PR?
--complete
command line flag to sqllogictestAre these changes tested?
not directly -- they are a tool for testing
As an example:
As an example, I ported two tests from aggregates.rs to aggregate.slt like:
Note I didn't put in the expected results
Then I ran in complete mode:
cargo test --test sqllogictests -- aggregate --complete
🎉
Are there any user-facing changes?
No