Simplify timing UX and remove complexity

Streamline timing summary to a single switch, enabling
full deterministic per-file timings sorted slowest-first.
Eliminate all mode and top-N options in sqllogictests.rs,
including the removal of TimingSummaryMode and related
auto branching for summary output. Update README.md
to recommend Unix post-processing with `| head -n 10`.

Author: kosiew

datafusion/sqllogictest/README.md

@@ -75,24 +75,22 @@ RUST_LOG=debug cargo test --test sqllogictests -- ddl
 The sqllogictest runner can emit deterministic per-file elapsed timings to help
 identify slow test files.
 
-By default (`--timing-summary auto`), timing summary output is disabled in local
-TTY runs and shows a top-slowest summary in non-TTY/CI runs.
-
-`--timing-top-n` / `SLT_TIMING_TOP_N` must be a positive integer (`>= 1`).
+Timing summary output is disabled by default and enabled with
+`--timing-summary` (or `SLT_TIMING_SUMMARY=1`).
 
 ```shell
-# Show top 10 slowest files (good for CI)
-cargo test --test sqllogictests -- --timing-summary top --timing-top-n 10
+# Show deterministic per-file elapsed timings (sorted slowest first)
+cargo test --test sqllogictests -- --timing-summary
 ```
 
 ```shell
-# Show full per-file timing table
-cargo test --test sqllogictests -- --timing-summary full
+# Keep only the top 10 lines using standard shell tooling
+cargo test --test sqllogictests -- --timing-summary | head -n 10
 ```
 
 ```shell
-# Same controls via environment variables
-SLT_TIMING_SUMMARY=top SLT_TIMING_TOP_N=15 cargo test --test sqllogictests
+# Enable via environment variable
+SLT_TIMING_SUMMARY=1 cargo test --test sqllogictests
 ```
 
 ```shell

datafusion/sqllogictest/bin/sqllogictests.rs

@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use clap::{ColorChoice, Parser, ValueEnum};
+use clap::{ColorChoice, Parser};
 use datafusion::common::instant::Instant;
 use datafusion::common::utils::get_available_parallelism;
 use datafusion::common::{DataFusionError, Result, exec_datafusion_err, exec_err};
@@ -61,14 +61,6 @@ const SQLITE_PREFIX: &str = "sqlite";
 const ERRS_PER_FILE_LIMIT: usize = 10;
 const TIMING_DEBUG_SLOW_FILES_ENV: &str = "SLT_TIMING_DEBUG_SLOW_FILES";
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
-enum TimingSummaryMode {
-    Auto,
-    Off,
-    Top,
-    Full,
-}
-
 #[derive(Debug)]
 struct FileTiming {
     relative_path: PathBuf,
@@ -321,7 +313,7 @@ async fn run_tests() -> Result<()> {
             .then_with(|| a.relative_path.cmp(&b.relative_path))
     });
 
-    print_timing_summary(&options, &m, is_ci, &file_timings)?;
+    print_timing_summary(&options, &m, &file_timings)?;
 
     let errors: Vec<_> = file_results
         .into_iter()
@@ -384,27 +376,14 @@ async fn run_tests() -> Result<()> {
 fn print_timing_summary(
     options: &Options,
     progress: &MultiProgress,
-    is_ci: bool,
     file_timings: &[FileTiming],
 ) -> Result<()> {
-    let mode = options.timing_summary_mode(is_ci);
-    if mode == TimingSummaryMode::Off || file_timings.is_empty() {
+    if !options.timing_summary || file_timings.is_empty() {
         return Ok(());
     }
 
-    let top_n = options.timing_top_n;
-    debug_assert!(matches!(
-        mode,
-        TimingSummaryMode::Top | TimingSummaryMode::Full
-    ));
-    let count = if mode == TimingSummaryMode::Full {
-        file_timings.len()
-    } else {
-        top_n
-    };
-
     progress.println("Per-file elapsed summary (deterministic):")?;
-    for (idx, timing) in file_timings.iter().take(count).enumerate() {
+    for (idx, timing) in file_timings.iter().enumerate() {
         progress.println(format!(
             "{:>3}. {:>8.3}s  {}",
             idx + 1,
@@ -413,13 +392,6 @@ fn print_timing_summary(
         ))?;
     }
 
-    if mode != TimingSummaryMode::Full && file_timings.len() > count {
-        progress.println(format!(
-            "... {} more files omitted (use --timing-summary full to show all)",
-            file_timings.len() - count
-        ))?;
-    }
-
     Ok(())
 }
 
@@ -434,16 +406,6 @@ fn is_env_truthy(name: &str) -> bool {
         })
 }
 
-fn parse_timing_top_n(arg: &str) -> std::result::Result<usize, String> {
-    let parsed = arg
-        .parse::<usize>()
-        .map_err(|error| format!("invalid value '{arg}': {error}"))?;
-    if parsed == 0 {
-        return Err("must be >= 1".to_string());
-    }
-    Ok(parsed)
-}
-
 async fn run_test_file_substrait_round_trip(
     test_file: TestFile,
     validator: Validator,
@@ -940,20 +902,10 @@ struct Options {
     #[clap(
         long,
         env = "SLT_TIMING_SUMMARY",
-        value_enum,
-        default_value_t = TimingSummaryMode::Auto,
-        help = "Per-file timing summary mode: auto|off|top|full"
-    )]
-    timing_summary: TimingSummaryMode,
-
-    #[clap(
-        long,
-        env = "SLT_TIMING_TOP_N",
-        default_value_t = 10,
-        value_parser = parse_timing_top_n,
-        help = "Number of files to show when timing summary mode is auto/top (must be >= 1)"
+        default_value_t = false,
+        help = "Print deterministic per-file timing summary"
     )]
-    timing_top_n: usize,
+    timing_summary: bool,
 
     #[clap(
         long,
@@ -965,19 +917,6 @@ struct Options {
 }
 
 impl Options {
-    fn timing_summary_mode(&self, is_ci: bool) -> TimingSummaryMode {
-        match self.timing_summary {
-            TimingSummaryMode::Auto => {
-                if is_ci {
-                    TimingSummaryMode::Top
-                } else {
-                    TimingSummaryMode::Off
-                }
-            }
-            mode => mode,
-        }
-    }
-
     /// Because this test can be run as a cargo test, commands like
     ///
     /// ```shell