findepi commented on code in PR #13672:
URL: https://github.com/apache/datafusion/pull/13672#discussion_r1874533180
##########
datafusion-cli/tests/integration_setup.bash:
##########
@@ -0,0 +1,16 @@
+# you should have localstack up, e.g by
+#$
LOCALSTACK_VERSION=sha256:a0b79cb2430f1818de2c66ce89d41bba40f5a1823410f5a7eaf3494b692eed97
+#$ podman run -d -p 4566:4566 localstack/localstack@$LOCALSTACK_VERSION
Review Comment:
arguably docker is kind of more standard way.
also, would it be possible not to require manual copy-pasting of the
commmands?
##########
datafusion-cli/tests/snapshots/cli_format_t...@automatic.snap:
##########
@@ -0,0 +1,21 @@
+---
+source: tests/cli_integration.rs
+info:
+ program: datafusion-cli
+ args:
+ - "--command"
+ - select 1
+ - "-q"
+ - "--format"
+ - automatic
+---
+success: true
+exit_code: 0
+----- stdout -----
++----------+
+| Int64(1) |
Review Comment:
How are these files generated / updated?
##########
datafusion-cli/tests/cli_integration.rs:
##########
@@ -17,42 +17,90 @@
use std::process::Command;
-use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt};
-use predicates::prelude::predicate;
use rstest::rstest;
+use insta::{glob, Settings};
+use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
+use std::{env, fs};
+
+fn cli() -> Command {
+ Command::new(get_cargo_bin("datafusion-cli"))
+}
+
+fn make_settings() -> Settings {
+ let mut settings = Settings::clone_current();
+ settings.set_prepend_module_to_snapshot(false);
+ settings
+}
+
#[cfg(test)]
#[ctor::ctor]
fn init() {
// Enable RUST_LOG logging configuration for tests
let _ = env_logger::try_init();
}
-// Disabled due to https://github.com/apache/datafusion/issues/10793
-#[cfg(not(target_family = "windows"))]
#[rstest]
-#[case::exec_from_commands(
- ["--command", "select 1", "--format", "json", "-q"],
- "[{\"Int64(1)\":1}]\n"
-)]
#[case::exec_multiple_statements(
- ["--command", "select 1; select 2;", "--format", "json", "-q"],
- "[{\"Int64(1)\":1}]\n[{\"Int64(2)\":2}]\n"
+ "statements",
+ ["--command", "select 1; select 2;", "-q"],
)]
#[case::exec_from_files(
- ["--file", "tests/data/sql.txt", "--format", "json", "-q"],
- "[{\"Int64(1)\":1}]\n"
+ "files",
+ ["--file", "tests/sql/select.sql", "-q"],
)]
#[case::set_batch_size(
- ["--command", "show datafusion.execution.batch_size", "--format", "json",
"-q", "-b", "1"],
- "[{\"name\":\"datafusion.execution.batch_size\",\"value\":\"1\"}]\n"
+ "batch_size",
+ ["--command", "show datafusion.execution.batch_size", "-q", "-b", "1"],
)]
#[test]
fn cli_quick_test<'a>(
+ #[case] snapshot_name: &'a str,
#[case] args: impl IntoIterator<Item = &'a str>,
- #[case] expected: &str,
) {
- let mut cmd = Command::cargo_bin("datafusion-cli").unwrap();
+ let mut settings = make_settings();
+ settings.set_snapshot_suffix(snapshot_name);
+ let _bound = settings.bind_to_scope();
+
+ let mut cmd = cli();
cmd.args(args);
- cmd.assert().stdout(predicate::eq(expected));
+
+ assert_cmd_snapshot!("cli_quick_test", cmd);
+}
+
+#[rstest]
+#[case("csv")]
+#[case("tsv")]
+#[case("table")]
+#[case("json")]
+#[case("nd-json")]
+#[case("automatic")]
+#[test]
+fn cli_format_test<'a>(#[case] format: &'a str) {
+ let mut settings = make_settings();
+ settings.set_snapshot_suffix(format);
+ let _bound = settings.bind_to_scope();
+
+ let mut cmd = cli();
+ cmd.args(["--command", "select 1", "-q", "--format", format]);
+
+ assert_cmd_snapshot!("cli_format_test", cmd);
+}
+
+#[test]
+fn test_storage_integration() {
+ if env::var("TEST_STORAGE_INTEGRATION").is_err() {
+ eprintln!("Skipping external storages integration tests");
+ return;
+ }
+
+ let mut settings = make_settings();
+ settings.add_filter(r"Elapsed .* seconds\.", "[ELAPSED]");
+ settings.add_filter(r"DataFusion CLI v.*", "[CLI_VERSION]");
+ let _bound = settings.bind_to_scope();
+
+ glob!("sql/*.sql", |path| {
+ let input = fs::read_to_string(path).unwrap();
+ assert_cmd_snapshot!("test_storage_integration",
cli().pass_stdin(input))
Review Comment:
would be nice to pull "test_storage_integration" out of the test name
automatically, to avoid copy-paste mistakes.
fwiw such functionality exists in the goldie crate (that's also for output
testing), and is as simple as
```rust
#[macro_export]
macro_rules! current_function_plain_name {
() => {{
fn f() {}
fn type_name_of_val<T>(_: T) -> &'static str {
::std::any::type_name::<T>()
}
let mut name = type_name_of_val(f).strip_suffix("::f").unwrap_or("");
while let Some(rest) = name.strip_suffix("::{{closure}}") {
name = rest;
}
name.split("::").last().unwrap_or(name)
}};
}
```
(admittedly hacky, but quite helpful)
--
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-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
