Skip to content

Commit

Permalink
fix the behavior of background system commands
Browse files Browse the repository at this point in the history
Signed-off-by: xxchan <[email protected]>
  • Loading branch information
xxchan committed Apr 17, 2024
1 parent fa4cc21 commit 521bdf2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* bin: When using `-j <jobs>` to run tests in parallel, add a random suffix to the temporary databases. This is useful if the test is manually canceled, but you want to rerun it freshly. Note that if the test failed, the database will be dropped. This is existing behavior and unchanged.
* bin: replace `env_logger` with `tracing-subscriber`. You will be able to see the record being executed with `RUST_LOG=debug sqllogictest ...`.
* runner: fix the behavior of background `system` commands (end with `&`). In `0.20.0`, it will block until the process exits. Now we return immediately.
```
system ok
sleep 5 &
```

## [0.20.0] - 2024-04-08

Expand Down
30 changes: 27 additions & 3 deletions sqllogictest/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,11 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
loc: _,
stdout: expected_stdout,
} => {
let command = match self.may_substitute(command) {
if should_skip(&self.labels, "", &conditions) {
return RecordOutput::Nothing;
}

let mut command = match self.may_substitute(command) {
Ok(command) => command,
Err(error) => {
return RecordOutput::System {
Expand All @@ -629,8 +633,14 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
}
};

if should_skip(&self.labels, "", &conditions) {
return RecordOutput::Nothing;
let is_background = command
.trim()
.chars()
.last()
.expect("system command is emptry")
== '&';
if is_background {
command = command.trim_end_matches('&').trim().to_string();
}

let mut cmd = if cfg!(target_os = "windows") {
Expand All @@ -642,8 +652,22 @@ impl<D: AsyncDB, M: MakeConnection<Conn = D>> Runner<D, M> {
cmd.arg("-c").arg(command);
cmd
};

if is_background {
// Spawn a new process, but don't wait for stdout, otherwise it will block until the process exits.
let error: Option<AnyError> = match cmd.spawn() {
Ok(_) => None,
Err(e) => Some(Arc::new(e)),
};
return RecordOutput::System {
error,
stdout: None,
};
}

cmd.stdout(std::process::Stdio::piped());
cmd.stderr(std::process::Stdio::piped());

let result = D::run_command(cmd).await;
#[derive(thiserror::Error, Debug)]
#[error(
Expand Down

0 comments on commit 521bdf2

Please sign in to comment.