Skip to content

Commit 08c09db

Browse files
authored
feat: support sqllogictest output coloring (#20368)
## Which issue does this PR close? - Closes #20367. ## Rationale for this change It's more ergonomic to have colored diffs in sqllogictest's output. The upstream library already supports it, and we can enable it based on the user's choice. This PR checks `NO_COLOR`, terminal settings, `CARGO_TERM_COLOR` and `--color` CLI argument. By default, the diff is colored. <img width="1243" height="338" alt="Screenshot 2026-02-15 at 11 44 05" src="https://github.com/user-attachments/assets/a0aec386-ff33-4935-9a58-ba96fcd4ceb8" /> ## What changes are included in this PR? - sqllogictest driver output colouring with argument/flags analysis ## Are these changes tested? - Tested different flag combinations ## Are there any user-facing changes? --------- Signed-off-by: theirix <theirix@gmail.com>
1 parent 5bfcf95 commit 08c09db

File tree

1 file changed

+57
-5
lines changed

1 file changed

+57
-5
lines changed

datafusion/sqllogictest/bin/sqllogictests.rs

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use clap::Parser;
18+
use clap::{ColorChoice, Parser};
1919
use datafusion::common::instant::Instant;
2020
use datafusion::common::utils::get_available_parallelism;
2121
use datafusion::common::{DataFusionError, Result, exec_datafusion_err, exec_err};
@@ -44,7 +44,9 @@ use datafusion::common::runtime::SpawnedTask;
4444
use futures::FutureExt;
4545
use std::ffi::OsStr;
4646
use std::fs;
47+
use std::io::{IsTerminal, stdout};
4748
use std::path::{Path, PathBuf};
49+
use std::str::FromStr;
4850

4951
#[cfg(feature = "postgres")]
5052
mod postgres_container;
@@ -123,6 +125,8 @@ async fn run_tests() -> Result<()> {
123125
.unwrap()
124126
.progress_chars("##-");
125127

128+
let colored_output = options.is_colored();
129+
126130
let start = Instant::now();
127131

128132
let test_files = read_test_files(&options)?;
@@ -176,6 +180,7 @@ async fn run_tests() -> Result<()> {
176180
m_style_clone,
177181
filters.as_ref(),
178182
currently_running_sql_tracker_clone,
183+
colored_output,
179184
)
180185
.await?
181186
}
@@ -187,6 +192,7 @@ async fn run_tests() -> Result<()> {
187192
m_style_clone,
188193
filters.as_ref(),
189194
currently_running_sql_tracker_clone,
195+
colored_output,
190196
)
191197
.await?
192198
}
@@ -294,6 +300,7 @@ async fn run_test_file_substrait_round_trip(
294300
mp_style: ProgressStyle,
295301
filters: &[Filter],
296302
currently_executing_sql_tracker: CurrentlyExecutingSqlTracker,
303+
colored_output: bool,
297304
) -> Result<()> {
298305
let TestFile {
299306
path,
@@ -323,7 +330,7 @@ async fn run_test_file_substrait_round_trip(
323330
runner.with_column_validator(strict_column_validator);
324331
runner.with_normalizer(value_normalizer);
325332
runner.with_validator(validator);
326-
let res = run_file_in_runner(path, runner, filters).await;
333+
let res = run_file_in_runner(path, runner, filters, colored_output).await;
327334
pb.finish_and_clear();
328335
res
329336
}
@@ -335,6 +342,7 @@ async fn run_test_file(
335342
mp_style: ProgressStyle,
336343
filters: &[Filter],
337344
currently_executing_sql_tracker: CurrentlyExecutingSqlTracker,
345+
colored_output: bool,
338346
) -> Result<()> {
339347
let TestFile {
340348
path,
@@ -364,7 +372,7 @@ async fn run_test_file(
364372
runner.with_column_validator(strict_column_validator);
365373
runner.with_normalizer(value_normalizer);
366374
runner.with_validator(validator);
367-
let result = run_file_in_runner(path, runner, filters).await;
375+
let result = run_file_in_runner(path, runner, filters, colored_output).await;
368376
pb.finish_and_clear();
369377
result
370378
}
@@ -373,6 +381,7 @@ async fn run_file_in_runner<D: AsyncDB, M: MakeConnection<Conn = D>>(
373381
path: PathBuf,
374382
mut runner: sqllogictest::Runner<D, M>,
375383
filters: &[Filter],
384+
colored_output: bool,
376385
) -> Result<()> {
377386
let path = path.canonicalize()?;
378387
let records =
@@ -386,7 +395,11 @@ async fn run_file_in_runner<D: AsyncDB, M: MakeConnection<Conn = D>>(
386395
continue;
387396
}
388397
if let Err(err) = runner.run_async(record).await {
389-
errs.push(format!("{err}"));
398+
if colored_output {
399+
errs.push(format!("{}", err.display(true)));
400+
} else {
401+
errs.push(format!("{err}"));
402+
}
390403
}
391404
}
392405

@@ -479,7 +492,7 @@ async fn run_test_file_with_postgres(
479492
runner.with_column_validator(strict_column_validator);
480493
runner.with_normalizer(value_normalizer);
481494
runner.with_validator(validator);
482-
let result = run_file_in_runner(path, runner, filters).await;
495+
let result = run_file_in_runner(path, runner, filters, false).await;
483496
pb.finish_and_clear();
484497
result
485498
}
@@ -772,6 +785,14 @@ struct Options {
772785
default_value_t = get_available_parallelism()
773786
)]
774787
test_threads: usize,
788+
789+
#[clap(
790+
long,
791+
value_name = "MODE",
792+
help = "Control colored output",
793+
default_value_t = ColorChoice::Auto
794+
)]
795+
color: ColorChoice,
775796
}
776797

777798
impl Options {
@@ -813,6 +834,37 @@ impl Options {
813834
eprintln!("WARNING: Ignoring `--show-output` compatibility option");
814835
}
815836
}
837+
838+
/// Determine if colour output should be enabled, respecting --color, NO_COLOR, CARGO_TERM_COLOR, and terminal detection
839+
fn is_colored(&self) -> bool {
840+
// NO_COLOR takes precedence
841+
if std::env::var_os("NO_COLOR").is_some() {
842+
return false;
843+
}
844+
845+
match self.color {
846+
ColorChoice::Always => true,
847+
ColorChoice::Never => false,
848+
ColorChoice::Auto => {
849+
// CARGO_TERM_COLOR takes precedence over auto-detection
850+
let cargo_term_color = ColorChoice::from_str(
851+
&std::env::var("CARGO_TERM_COLOR")
852+
.unwrap_or_else(|_| "auto".to_string()),
853+
)
854+
.unwrap_or(ColorChoice::Auto);
855+
match cargo_term_color {
856+
ColorChoice::Always => true,
857+
ColorChoice::Never => false,
858+
ColorChoice::Auto => {
859+
// Auto for both CLI argument and CARGO_TERM_COLOR,
860+
// then use colors by default for non-dumb terminals
861+
stdout().is_terminal()
862+
&& std::env::var("TERM").unwrap_or_default() != "dumb"
863+
}
864+
}
865+
}
866+
}
867+
}
816868
}
817869

818870
/// Performs scratch file check for all test files.

0 commit comments

Comments
 (0)