alamb commented on code in PR #4735:
URL: https://github.com/apache/arrow-datafusion/pull/4735#discussion_r1057279058
##########
datafusion/core/tests/parquet/filter_pushdown.rs:
##########
@@ -51,7 +52,7 @@ fn init() {
}
#[cfg(not(target_family = "windows"))]
-#[tokio::test]
+#[tokio::test(flavor = "multi_thread")]
Review Comment:
```suggestion
// Use multi-threaded executor as this test consumes CPU
#[tokio::test(flavor = "multi_thread")]
```
##########
datafusion/core/tests/parquet/filter_pushdown.rs:
##########
@@ -226,9 +219,11 @@ async fn single_file() {
.unwrap(),
)
.with_pushdown_expected(PushdownExpected::None)
- .with_expected_rows(NUM_ROWS)
- .run()
- .await;
+ .with_expected_rows(NUM_ROWS);
+ set.spawn(async move { case.run().await });
+
+ // Join all the cases.
+ while let Some(_) = set.join_next().await {}
Review Comment:
I think this also needs to check the result for Err (which happens when the
task panic's). Something like
```suggestion
while let Some(result) = set.join_next().await {
result.unwrap()
}
```
?
As written, when I artificially make one of the tests panic like
```diff
--- a/datafusion/core/tests/parquet/filter_pushdown.rs
+++ b/datafusion/core/tests/parquet/filter_pushdown.rs
@@ -188,7 +188,7 @@ async fn single_file() {
)
.with_pushdown_expected(PushdownExpected::Some)
.with_expected_rows(88);
- set.spawn(async move { case.run().await });
+ set.spawn(async move { case.run().await; panic!("DOO") });
let case = TestCase::new(test_parquet_file.clone())
.with_name("dict_disjunction")
```
Then `cargo test -p datafusion --test parquet_exec` still passes
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
