chore: Overhaul CLI tests for better ergonomics.#26168
chore: Overhaul CLI tests for better ergonomics.#26168jacksonrnewhouse merged 2 commits intomainfrom
Conversation
jacksonrnewhouse
force-pushed
the
improve_cli_testing
branch
from
cc88afa to
0b59d1a
Compare
| .await | ||
| .json::<Value>() | ||
| .await | ||
| .query_sql("foo") |
There was a problem hiding this comment.
nit: The semantics of this bother me a bit it reads as "query with query" which feels weird rolling off the mental togue. What do you think about .query(<database_name>).with_sql(<sql_string>) instead? Then for influxql queries we would have .query(<database_name>).with_influxql(<influxql_string>)?
There was a problem hiding this comment.
Actually, I just realize I was confusing myself -- I thought this change was happening in the influxdb3_client crate and was hoping to see an improvement in the semantics of that API 🤦. Thanks for making the requested change!
waynr
left a comment
waynr
left a comment
There was a problem hiding this comment.
This looks good to me for what it's doing (improving ergonomics of writing tests, making it easier to add standard arguments across many tests at once), although I have a minor preference as a reader of test cases for actually seeing the CLI args in plain form.
3 participants
We need some additional multi-node testing for the Enterprise multi-node system. The CLI tests so far have just use
run()orrun_with_confirmation(), which requires implementers to correctly construct the CLI commands. This change introduces a variety of Query structs that have arun()command.