Skip to content

Testable process termination#48

Open
nfnt wants to merge 1 commit into
alphafrom
nfnt/test-process-termination
Open

Testable process termination#48
nfnt wants to merge 1 commit into
alphafrom
nfnt/test-process-termination

Conversation

@nfnt

@nfnt nfnt commented Jul 29, 2025

Copy link
Copy Markdown
Contributor

Moves process handling of the executor into a separate function. This
allows to have tests for process termination.

@nfnt
nfnt requested a review from orlandohohmeier July 29, 2025 12:39
@nfnt
nfnt force-pushed the nfnt/test-process-termination branch from afa585a to 550562b Compare July 29, 2025 12:44
@codecov

codecov Bot commented Jul 29, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.05882% with 28 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/worker/src/executor/process.rs 67.05% 28 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread crates/worker/src/driver/process.rs Outdated
sleep(Duration::from_secs(1)).await;

token.cancel();
process.await.unwrap();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wonder whether we should wrap this in a timeout as this shouldn't hang

Moves process handling of the executor into a separate function. This
allows to have tests for process termination.
@nfnt
nfnt force-pushed the nfnt/test-process-termination branch from 550562b to 996b440 Compare September 26, 2025 11:57
@juliangieseke
juliangieseke changed the base branch from main to alpha November 6, 2025 14:40
@juliangieseke
juliangieseke dismissed orlandohohmeier’s stale review November 6, 2025 14:40

The base branch was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants