Skip to content

No cargo test for sort_mem_validation #21222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blaginin merged 1 commit into from
Mar 29, 2026
+7 −47
Merged

No cargo test for sort_mem_validation #21222

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 7 additions & 47 deletions datafusion/core/tests/memory_limit/memory_limit_validation/sort_mem_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,10 @@
//! This file is organized as:
//! - Test runners that spawn individual test processes
//! - Test cases that contain the actual validation logic
use log::info;
use std::sync::Once;
use std::{process::Command, str};

use crate::memory_limit::memory_limit_validation::utils;

static INIT: Once = Once::new();

// ===========================================================================
// Test runners:
// Runners are split into multiple tests to run in parallel
Expand Down Expand Up @@ -69,62 +65,26 @@ fn sort_with_mem_limit_2_cols_2_runner() {
spawn_test_process("sort_with_mem_limit_2_cols_2");
}

/// `spawn_test_process` might trigger multiple recompilations and the test binary
/// size might grow indefinitely. This initializer ensures recompilation is only done
/// once and the target size is bounded.
///
/// TODO: This is a hack, can be cleaned up if we have a better way to let multiple
/// test cases run in different processes (instead of different threads by default)
fn init_once() {
INIT.call_once(|| {
let _ = Command::new("cargo")
.arg("test")
.arg("--no-run")
.arg("--package")
.arg("datafusion")
.arg("--test")
.arg("core_integration")
.arg("--features")
.arg("extended_tests")
.env("DATAFUSION_TEST_MEM_LIMIT_VALIDATION", "1")
.output()
.expect("Failed to execute test command");
});
}

/// Helper function that executes a test in a separate process with the required environment
/// variable set. Memory limit validation tasks need to measure memory resident set
/// size (RSS), so they must run in a separate process.
/// Helper function that executes a test in a separate process with the required
/// environment variable set. Re-invokes the current test binary directly,
/// avoiding cargo overhead and recompilation.
fn spawn_test_process(test: &str) {
init_once();

let test_path =
format!("memory_limit::memory_limit_validation::sort_mem_validation::{test}");
info!("Running test: {test_path}");

// Run the test command
let output = Command::new("cargo")
.arg("test")
.arg("--package")
.arg("datafusion")
.arg("--test")
.arg("core_integration")
.arg("--features")
.arg("extended_tests")
.arg("--")

let exe = std::env::current_exe().expect("Failed to get test binary path");
Copy link
Copy Markdown
Contributor

@2010YOUY01 2010YOUY01 Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL


let output = Command::new(exe)
.arg(&test_path)
.arg("--exact")
.arg("--nocapture")
.env("DATAFUSION_TEST_MEM_LIMIT_VALIDATION", "1")
.output()
.expect("Failed to execute test command");

// Convert output to strings
let stdout = str::from_utf8(&output.stdout).unwrap_or("");
let stderr = str::from_utf8(&output.stderr).unwrap_or("");

info!("{stdout}");

assert!(
output.status.success(),
"Test '{}' failed with status: {}\nstdout:\n{}\nstderr:\n{}",
Expand Down
Loading