You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "blaginin (via GitHub)" <gi...@apache.org> on 2026/03/28 22:49:39 UTC
[PR] No cargo test for `sort_mem_validation` [datafusion]
blaginin opened a new pull request, #21222:
URL: https://github.com/apache/datafusion/pull/21222
Every time I run tests with `sort_mem_validation` i'm forced to recompile those tests several times, as those trigger `cargo test` (=recompilation with different features). I made it use the existing compiled binary
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] No cargo test for `sort_mem_validation` [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin merged PR #21222:
URL: https://github.com/apache/datafusion/pull/21222
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] No cargo test for `sort_mem_validation` [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on code in PR #21222:
URL: https://github.com/apache/datafusion/pull/21222#discussion_r3005474834
##########
datafusion/core/tests/memory_limit/memory_limit_validation/sort_mem_validation.rs:
##########
@@ -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");
Review Comment:
TIL
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] No cargo test for `sort_mem_validation` [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on PR #21222:
URL: https://github.com/apache/datafusion/pull/21222#issuecomment-4149607147
Run extended tests
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] No cargo test for `sort_mem_validation` [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on PR #21222:
URL: https://github.com/apache/datafusion/pull/21222#issuecomment-4149636554
I think this runs 2-3 minutes faster then on develop (no need to recompile code twice) 🚀
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org