Skip to content

Minor: Assert test_enabled_backtrace requirements to run #11525

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

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

comphead
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

If the test_enabled_backtrace launched without RUST_BACKTRACE=1 the test will fail with the reason which require some time to investigate.

What changes are included in this PR?

Add proper error message if assertion doesn't satisfy prior to test run

Are these changes tested?

Are there any user-facing changes?

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

I verified that when i run

cargo test -p datafusion-common --all-features

With this branch it fails with a useful message:

---- error::test::test_enabled_backtrace stdout ----
thread 'error::test::test_enabled_backtrace' panicked at datafusion/common/src/error.rs:671:18:
Environment variable RUST_BACKTRACE must be set to 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

And it runs well with that variable set:

andrewlamb@Andrews-MBP-2:~/Software/datafusion$ RUST_BACKTRACE=1 cargo test -p datafusion-common --all-features -- backtrace
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.10s
     Running unittests src/lib.rs (target/debug/deps/datafusion_common-87ffe852e4faebf2)

running 1 test
test error::test::test_enabled_backtrace ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 205 filtered out; finished in 0.03s

   Doc-tests datafusion_common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 42 filtered out; finished in 0.00s

On main the error is not helpful:

andrewlamb@Andrews-MBP-2:~/Software/datafusion$ cargo test -p datafusion-common --all-features -- backtrace
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.09s
     Running unittests src/lib.rs (target/debug/deps/datafusion_common-87ffe852e4faebf2)

running 1 test
test error::test::test_enabled_backtrace ... FAILED

failures:

---- error::test::test_enabled_backtrace stdout ----
thread 'error::test::test_enabled_backtrace' panicked at datafusion/common/src/error.rs:671:9:
assertion failed: err.contains(DataFusionError::BACK_TRACE_SEP)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    error::test::test_enabled_backtrace

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 209 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p datafusion-common --lib`

fyi @findepi

#[cfg(feature = "backtrace")]
#[test]
#[allow(clippy::unnecessary_literal_unwrap)]
fn test_enabled_backtrace() {
match std::env::var("RUST_BACKTRACE") {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@comphead comphead merged commit 1b9b35c into apache:main Jul 17, 2024
23 checks passed
@findepi
Copy link
Member

findepi commented Jul 17, 2024

thank you!

Lordworms pushed a commit to Lordworms/arrow-datafusion that referenced this pull request Jul 23, 2024
wiedld pushed a commit to influxdata/arrow-datafusion that referenced this pull request Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants