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

Move assert_batches_eq! macros to test_utils.rs #746

Merged
merged 2 commits into from
Jul 19, 2021

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jul 18, 2021

Which issue does this PR close?

Closes #745, re #743

Rationale for this change

The assert_batches_eq! and assert_batches_sorted_eq! macros are used to write easier to maintain tests. However, the macros are in the test module that is only compiled for tests of the datafusion crate and are not, therefore, available to the "integration" style tests in sql.rs

#[cfg(test)]
pub mod test;

I also find them so useful they are also in IOx in test_utils.rs

BTW the reason they are "easier to maintain tests" is that you can copy the output directly from the test failure into the code to update the results (rather than having to reformat it manually). For example, to generate the new output in sql.rs, I simply put a placeholder in and ran the test and then copy/pasted in the output of the test:

---- parquet_query stdout ----
thread 'parquet_query' panicked at 'assertion failed: `(left == right)`
  left: `["foo"]`,
 right: `["+----+--------------------------+", "| id | CAST(string_col AS Utf8) |", "+----+--------------------------+", "| 4  | 0                        |", "| 5  | 1                        |", "| 6  | 0                        |", "| 7  | 1                        |", "| 2  | 0                        |", "| 3  | 1                        |", "| 0  | 0                        |", "| 1  | 1                        |", "+----+--------------------------+"]`: 

expected:

[
    "foo",
]
actual:

[
    "+----+--------------------------+",
    "| id | CAST(string_col AS Utf8) |",
    "+----+--------------------------+",
    "| 4  | 0                        |",
    "| 5  | 1                        |",
    "| 6  | 0                        |",
    "| 7  | 1                        |",
    "| 2  | 0                        |",
    "| 3  | 1                        |",
    "| 0  | 0                        |",
    "| 1  | 1                        |",
    "+----+--------------------------+",
]

', datafusion/tests/sql.rs:121:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

What changes are included in this PR?

  1. Move the he assert_batches_eq! and assert_batches_sorted_eq! macros to test_utils.rs
  2. port one test in sql.rs to demonstrate their use

Are there any user-facing changes?

The assert_batches_eq! and assert_batches_sorted_eq! macros are now part of the DataFusion public API

Follow on work is tracked in #743

@github-actions github-actions bot added the datafusion Changes in the datafusion crate label Jul 18, 2021
@alamb alamb marked this pull request as ready for review July 18, 2021 11:55
@alamb alamb requested review from andygrove and Dandandan July 18, 2021 12:03
@alamb alamb merged commit 8a17c18 into apache:master Jul 19, 2021
@alamb alamb deleted the alamb/move_assert_batches branch October 6, 2022 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move the assert_batches_eq! macros to a non part of datafusion
2 participants