Skip to content
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 snapshot testing to CLI & set up AWS mock #13672

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2887bb9
Do not normalize values
blaginin Nov 26, 2024
813d634
Fix tests & update docs
blaginin Nov 26, 2024
c3de620
Prettier
blaginin Nov 26, 2024
0e11d36
Merge branch 'main' into bugfix/do-not-normalize-values
blaginin Dec 1, 2024
cb7da8c
Merge branch 'main' into bugfix/do-not-normalize-values
blaginin Dec 2, 2024
7c2b3fe
Lowercase config params
blaginin Dec 3, 2024
9146e4b
Add snap to CLI & set up AWS mock
blaginin Dec 5, 2024
9d856c3
Refactor tests
blaginin Dec 6, 2024
0574ab8
Unify transform and parse
blaginin Dec 9, 2024
2045495
Merge branch 'main' into bugfix/do-not-normalize-values
blaginin Dec 9, 2024
06c013d
Fix tests
blaginin Dec 9, 2024
4870219
Merge branch 'main' into cli-snap
blaginin Dec 10, 2024
a02625e
Merge branch 'bugfix/do-not-normalize-values' into cli-snap
blaginin Dec 10, 2024
65809a7
Setup CLI
blaginin Dec 10, 2024
05a562f
Show minio output
blaginin Dec 10, 2024
36f8550
Format Cargo.toml
blaginin Dec 10, 2024
921f229
Do not hardcode AWS params
blaginin Dec 11, 2024
107c515
Test options parsing
blaginin Dec 11, 2024
2d430a0
Add allow http
blaginin Dec 11, 2024
4b56506
Fix aws build
blaginin Dec 12, 2024
1581e7a
Merge branch 'main' into cli-snap
blaginin Dec 12, 2024
ad9734e
Fix ip
blaginin Dec 13, 2024
0b24daa
Remove slash ☠️
blaginin Dec 13, 2024
5f29c00
Format cargo toml
blaginin Dec 13, 2024
a3826d4
Remove integration_setup.bash
blaginin Dec 13, 2024
9cb1c99
Update docs
blaginin Dec 13, 2024
53c9c51
Do not hardcode test names
blaginin Dec 13, 2024
eb87f60
Merge branch 'main' into cli-snap
blaginin Dec 17, 2024
855315f
Relock cargo
blaginin Dec 17, 2024
fd3240d
Merge branch 'main' into cli-snap
blaginin Dec 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions datafusion-cli/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions datafusion-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,5 @@ assert_cmd = "2.0"
ctor = "0.2.0"
predicates = "3.0"
rstest = "0.22"
insta = { version = "1.41.1", features = ["glob", "filters"] }
insta-cmd = "0.6.0"
82 changes: 65 additions & 17 deletions datafusion-cli/tests/cli_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#[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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, fixed in 53c9c51

});
}
16 changes: 16 additions & 0 deletions datafusion-cli/tests/integration_setup.bash
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arguably docker is kind of more standard way.

also, would it be possible not to require manual copy-pasting of the commmands?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#$ podman run -d -p 1338:1338 amazon/amazon-ec2-metadata-mock:v1.9.2 --imdsv2

export TEST_INTEGRATION=1
export AWS_DEFAULT_REGION=us-east-1
export AWS_ACCESS_KEY_ID=test
export AWS_SECRET_ACCESS_KEY=test
export AWS_ENDPOINT=http://localhost:4566
export AWS_ALLOW_HTTP=true
export AWS_BUCKET_NAME=test-bucket


aws s3 mb s3://test-bucket --endpoint-url=$AWS_ENDPOINT
aws s3 cp ../datafusion/core/tests/data/cars.csv s3://test-bucket/cars.csv --endpoint-url=$AWS_ENDPOINT
21 changes: 21 additions & 0 deletions datafusion-cli/tests/snapshots/cli_format_test@automatic.snap
Original file line number Diff line number Diff line change
@@ -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) |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are these files generated / updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+----------+
| 1 |
+----------+

----- stderr -----
18 changes: 18 additions & 0 deletions datafusion-cli/tests/snapshots/cli_format_test@csv.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: tests/cli_integration.rs
info:
program: datafusion-cli
args:
- "--command"
- select 1
- "-q"
- "--format"
- csv
---
success: true
exit_code: 0
----- stdout -----
Int64(1)
1

----- stderr -----
17 changes: 17 additions & 0 deletions datafusion-cli/tests/snapshots/cli_format_test@json.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: tests/cli_integration.rs
info:
program: datafusion-cli
args:
- "--command"
- select 1
- "-q"
- "--format"
- json
---
success: true
exit_code: 0
----- stdout -----
[{"Int64(1)":1}]

----- stderr -----
17 changes: 17 additions & 0 deletions datafusion-cli/tests/snapshots/cli_format_test@nd-json.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: tests/cli_integration.rs
info:
program: datafusion-cli
args:
- "--command"
- select 1
- "-q"
- "--format"
- nd-json
---
success: true
exit_code: 0
----- stdout -----
{"Int64(1)":1}

----- stderr -----
21 changes: 21 additions & 0 deletions datafusion-cli/tests/snapshots/cli_format_test@table.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: tests/cli_integration.rs
info:
program: datafusion-cli
args:
- "--command"
- select 1
- "-q"
- "--format"
- table
---
success: true
exit_code: 0
----- stdout -----
+----------+
| Int64(1) |
+----------+
| 1 |
+----------+

----- stderr -----
18 changes: 18 additions & 0 deletions datafusion-cli/tests/snapshots/cli_format_test@tsv.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: tests/cli_integration.rs
info:
program: datafusion-cli
args:
- "--command"
- select 1
- "-q"
- "--format"
- tsv
---
success: true
exit_code: 0
----- stdout -----
Int64(1)
1

----- stderr -----
21 changes: 21 additions & 0 deletions datafusion-cli/tests/snapshots/cli_quick_test@batch_size.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: tests/cli_integration.rs
info:
program: datafusion-cli
args:
- "--command"
- show datafusion.execution.batch_size
- "-q"
- "-b"
- "1"
---
success: true
exit_code: 0
----- stdout -----
+---------------------------------+-------+
| name | value |
+---------------------------------+-------+
| datafusion.execution.batch_size | 1 |
+---------------------------------+-------+

----- stderr -----
Loading
Loading