alamb commented on code in PR #12832:
URL: https://github.com/apache/datafusion/pull/12832#discussion_r1793876484
##########
datafusion/core/tests/fuzz_cases/aggregation_fuzzer/fuzzer.rs:
##########
@@ -237,45 +252,53 @@ struct AggregationFuzzTestTask {
}
impl AggregationFuzzTestTask {
- async fn run(&self) {
+ async fn run(&self) -> Result<()> {
let task_result = run_sql(&self.sql, &self.ctx_with_params.ctx)
.await
- .expect("should success to run sql");
- self.check_result(&task_result, &self.expected_result);
+ .map_err(|e| e.context(self.context_error_report()))?;
+ self.check_result(&task_result, &self.expected_result)
}
- // TODO: maybe we should persist the `expected_result` and `task_result`,
- // because the readability is not so good if we just print it.
- fn check_result(&self, task_result: &[RecordBatch], expected_result:
&[RecordBatch]) {
- let result = check_equality_of_batches(task_result, expected_result);
- if let Err(e) = result {
+ fn check_result(
+ &self,
+ task_result: &[RecordBatch],
+ expected_result: &[RecordBatch],
+ ) -> Result<()> {
+ check_equality_of_batches(task_result, expected_result).map_err(|e| {
// If we found inconsistent result, we print the test details for
reproducing at first
- println!(
- "##### AggregationFuzzer error report #####
- ### Sql:\n{}\n\
- ### Schema:\n{}\n\
- ### Session context params:\n{:?}\n\
- ### Inconsistent row:\n\
- - row_idx:{}\n\
- - task_row:{}\n\
- - expected_row:{}\n\
- ### Task total result:\n{}\n\
- ### Expected total result:\n{}\n\
- ### Input:\n{}\n\
- ",
- self.sql,
- self.dataset_ref.batches[0].schema_ref(),
- self.ctx_with_params.params,
+ let message = format!(
+ "{}\n\
+ ### Inconsistent row:\n\
+ - row_idx:{}\n\
+ - task_row:{}\n\
+ - expected_row:{}\n\
+ ### Task total result:\n{}\n\
+ ### Expected total result:\n{}\n\
+ ",
+ self.context_error_report(),
e.row_idx,
e.lhs_row,
e.rhs_row,
pretty_format_batches(task_result).unwrap(),
pretty_format_batches(expected_result).unwrap(),
- pretty_format_batches(&self.dataset_ref.batches).unwrap(),
);
+ DataFusionError::Internal(message)
+ })
+ }
- // Then we just panic
- panic!();
- }
+ /// Returns a formatted error message
+ fn context_error_report(&self) -> String {
+ format!(
+ "##### AggregationFuzzer error report #####\n\
+ ### Sql:\n{}\n\
+ ### Schema:\n{}\n\
+ ### Session context params:\n{:?}\n\
Review Comment:
I think that information is already included when inconsistent results are
detected (in `check_result`) but perhaps I don't understand what you are
proposing here
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
