-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
[sqllogictest] Run tests on Windows #5870
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ | |
|
||
use std::error::Error; | ||
use std::path::{Path, PathBuf}; | ||
#[cfg(target_family = "windows")] | ||
use std::thread; | ||
|
||
use log::info; | ||
use sqllogictest::strict_column_validator; | ||
|
@@ -33,16 +35,31 @@ mod utils; | |
const TEST_DIRECTORY: &str = "tests/sqllogictests/test_files/"; | ||
const PG_COMPAT_FILE_PREFIX: &str = "pg_compat_"; | ||
|
||
#[tokio::main] | ||
#[cfg(target_family = "windows")] | ||
pub async fn main() -> Result<(), Box<dyn Error>> { | ||
println!("Skipping test on windows"); | ||
Ok(()) | ||
pub fn main() { | ||
// Tests from `tpch.slt` fail with stackoverflow with the default stack size. | ||
thread::Builder::new() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In case somebody with more understanding takes a closer look, I added a stack trace for the overflow as well as a test to reproduce the issue with unit tests on any platform here https://gist.github.com/melgenek/b6acf4467ee10c1941f3b103a17e1b1e. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have hit this before (e.g. #4065) -- I think cranking up the default size is fine. Specifically, release builds seem to use much less stack so this is primarily a test issue rather than a production issue |
||
.stack_size(2 * 1024 * 1024) // 2 MB | ||
.spawn(move || { | ||
tokio::runtime::Builder::new_current_thread() | ||
.enable_all() | ||
.build() | ||
.unwrap() | ||
.block_on(async { run_tests().await }) | ||
.unwrap() | ||
}) | ||
.unwrap() | ||
.join() | ||
.unwrap(); | ||
} | ||
|
||
#[tokio::main] | ||
#[cfg(not(target_family = "windows"))] | ||
pub async fn main() -> Result<(), Box<dyn Error>> { | ||
run_tests().await | ||
} | ||
|
||
async fn run_tests() -> Result<(), Box<dyn Error>> { | ||
// Enable logging (e.g. set RUST_LOG=debug to see debug logs) | ||
env_logger::init(); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This piece was needed to have workarounds for creating tables without values. It is not needed anymore.