[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6429: Do not run avro sqllogictests tests unless the avro feature is enabled

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #6429:
URL: https://github.com/apache/arrow-datafusion/pull/6429#discussion_r1202113058


##########
datafusion/core/tests/sqllogictests/src/main.rs:
##########
@@ -84,7 +84,10 @@ async fn run_test_file(
     relative_path: PathBuf,
 ) -> Result<(), Box<dyn Error>> {
     info!("Running with DataFusion runner: {}", path.display());
-    let test_ctx = context_for_test_file(&relative_path).await;
+    let Some(test_ctx) = context_for_test_file(&relative_path).await else {
+        info!("Skipping: {}", path.display());

Review Comment:
   Yes, it will log twice. I was thinking that I wanted to log the reason the test was skipped (in `context_for_test_file` ) as well as make double sure that the logs showed any test that was skipped (if/when we add more tests that are skipped)
   
   Given that the logs aren't printed by default (you need to do `RUST_LOG=...` to do so ) I chose the "mo logs mo better" approach, but I can see how others might have a different preference
   
   I don't feel strongly and will change it if others prefer less logging



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org