From 464346faa5ca48f5e75641e0d4342cf34ba29873 Mon Sep 17 00:00:00 2001 From: Rustam Zaitov Date: Thu, 10 Sep 2026 18:45:21 +0200 Subject: [PATCH 1/4] test: cover process cleanup after task completion Add a local API integration scenario that completes a Bash task while a signal-resistant descendant keeps inherited output pipes open. Run a follow-up task in the same container to verify that the descendant was removed and clean up any unexpected survivor. Also assert successful, prompt completion and preserved stdout and stderr markers. --- .../SemaphoreProcessCleanupFixtures.java | 104 ++++++++++++++++++ .../junit/StepsParameterResolver.java | 6 + .../semaphore/LocalProcessCleanupTest.java | 90 +++++++++++++++ .../fixtures/bash/process-cleanup/README.md | 42 +++++++ .../fixtures/bash/process-cleanup/main.sh | 31 ++++++ .../bash/process-cleanup/resistant-process.sh | 19 ++++ .../fixtures/bash/process-cleanup/verify.sh | 24 ++++ 7 files changed, 316 insertions(+) create mode 100644 src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java create mode 100644 src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java create mode 100644 test-environment/fixtures/bash/process-cleanup/README.md create mode 100644 test-environment/fixtures/bash/process-cleanup/main.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/resistant-process.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/verify.sh diff --git a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java new file mode 100644 index 0000000..dcebffd --- /dev/null +++ b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java @@ -0,0 +1,104 @@ +package io.bookwright.fixtures.semaphore; + +import io.bookwright.api.model.semaphore.AccessKeyRequest; +import io.bookwright.api.model.semaphore.InventoryRequest; +import io.bookwright.api.model.semaphore.ProjectRequest; +import io.bookwright.api.model.semaphore.RepositoryRequest; +import io.bookwright.api.model.semaphore.TaskRequest; +import io.bookwright.api.model.semaphore.TemplateRequest; +import io.bookwright.config.MainConfig; +import io.bookwright.util.TestData; +import java.time.Duration; +import java.util.List; + +/** Typed data and expectations for local process-group cleanup. */ +public record SemaphoreProcessCleanupFixtures( + ProjectRequest project, + AccessKey accessKey, + Repository repository, + Inventory inventory, + Templates templates, + Expectations expectations) { + + public static SemaphoreProcessCleanupFixtures from(MainConfig config, TestData data) { + String suffix = Long.toUnsignedString(data.testSeed(), 36); + return new SemaphoreProcessCleanupFixtures( + new ProjectRequest("bookwright-process-cleanup-" + suffix, false, 0), + new AccessKey("bookwright-process-cleanup-key-" + suffix, "none"), + new Repository( + "bookwright-process-cleanup-repository-" + suffix, + config.fixturesRepository(), + config.fixturesDefaultBranch()), + new Inventory( + "bookwright-process-cleanup-inventory-" + suffix, + "[local]\nlocalhost ansible_connection=local", + "static"), + new Templates( + new Template( + "bookwright-process-cleanup-main-" + suffix, + "test-environment/fixtures/bash/process-cleanup/main.sh", + false), + new Template( + "bookwright-process-cleanup-verifier-" + suffix, + "test-environment/fixtures/bash/process-cleanup/verify.sh", + true)), + new Expectations( + "success", + "semaphore-process-cleanup-stdout-marker", + "semaphore-process-cleanup-stderr-marker", + "semaphore-process-cleanup-child-gone", + Duration.ofSeconds(30))); + } + + public TaskRequest verificationRequest(long verifierTemplateId, long completedTaskId) { + return new TaskRequest( + verifierTemplateId, null, null, "[\"%d\"]".formatted(completedTaskId), null, null); + } + + public record AccessKey(String name, String type) { + public AccessKeyRequest request(long projectId) { + return new AccessKeyRequest(name, type, projectId); + } + } + + public record Repository(String name, String gitUrl, String gitBranch) { + public RepositoryRequest request(long projectId, long keyId) { + return new RepositoryRequest(name, projectId, gitUrl, gitBranch, keyId); + } + } + + public record Inventory(String name, String content, String type) { + public InventoryRequest request(long projectId, long keyId) { + return new InventoryRequest(name, projectId, content, keyId, type); + } + } + + public record Template(String name, String playbook, boolean allowTaskArguments) { + public TemplateRequest request(long projectId, long repositoryId, long inventoryId) { + return new TemplateRequest( + name, + projectId, + inventoryId, + repositoryId, + 0, + playbook, + "bash", + "", + null, + allowTaskArguments, + List.of(), + null, + null, + false); + } + } + + public record Templates(Template main, Template verifier) {} + + public record Expectations( + String successfulTaskStatus, + String stdoutMarker, + String stderrMarker, + String childGoneMarker, + Duration maximumCompletionTime) {} +} diff --git a/src/main/java/io/bookwright/junit/StepsParameterResolver.java b/src/main/java/io/bookwright/junit/StepsParameterResolver.java index f4807be..78fa4ab 100644 --- a/src/main/java/io/bookwright/junit/StepsParameterResolver.java +++ b/src/main/java/io/bookwright/junit/StepsParameterResolver.java @@ -21,6 +21,7 @@ import io.bookwright.fixtures.semaphore.SemaphoreLdapFixtures; import io.bookwright.fixtures.semaphore.SemaphoreLoginSecurityFixtures; import io.bookwright.fixtures.semaphore.SemaphoreOidcFixtures; +import io.bookwright.fixtures.semaphore.SemaphoreProcessCleanupFixtures; import io.bookwright.fixtures.semaphore.SemaphoreProjectDeletionFixtures; import io.bookwright.fixtures.semaphore.SemaphoreRunnerRoutingFixtures; import io.bookwright.fixtures.semaphore.SemaphoreScheduleFixtures; @@ -75,6 +76,7 @@ public boolean supportsParameter( || type == SemaphoreLdapFixtures.class || type == SemaphoreLoginSecurityFixtures.class || type == SemaphoreOidcFixtures.class + || type == SemaphoreProcessCleanupFixtures.class || type == SemaphoreProjectDeletionFixtures.class || type == SemaphoreRunnerRoutingFixtures.class || type == SemaphoreScheduleFixtures.class @@ -151,6 +153,10 @@ public Object resolveParameter( if (type == SemaphoreOidcFixtures.class) { return SemaphoreOidcFixtures.standard(); } + if (type == SemaphoreProcessCleanupFixtures.class) { + return SemaphoreProcessCleanupFixtures.from( + io.bookwright.config.Configs.main(), TestDataExtension.getOrCreate(extensionContext)); + } if (type == SemaphoreProjectDeletionFixtures.class) { return SemaphoreProjectDeletionFixtures.from( io.bookwright.config.Configs.main(), TestDataExtension.getOrCreate(extensionContext)); diff --git a/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java b/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java new file mode 100644 index 0000000..2d5b200 --- /dev/null +++ b/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java @@ -0,0 +1,90 @@ +package io.bookwright.tests.semaphore; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.bookwright.annotations.Api; +import io.bookwright.annotations.OwnerDanil; +import io.bookwright.api.model.semaphore.Template; +import io.bookwright.fixtures.semaphore.SemaphoreProcessCleanupFixtures; +import io.bookwright.junit.Precondition; +import io.bookwright.junit.Preconditions; +import io.bookwright.steps.ApiSteps; +import io.qameta.allure.Feature; +import java.time.Duration; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; + +@Api +@OwnerDanil +@Feature("Semaphore local process cleanup") +@EnabledIfSystemProperty(named = "SEMAPHORE_PROFILE", matches = "core-sqlite-local") +class LocalProcessCleanupTest { + + @Test + @Preconditions(Precondition.SEMAPHORE_ADMIN_SESSION) + @DisplayName("Normal task completion cleans up a background descendant") + void normalCompletionCleansUpBackgroundDescendant( + ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + var templates = createTemplates(api, fixtures); + + long startedAt = System.nanoTime(); + var completedTask = + api.semaphore().tasks().startAndWait(templates.main().projectId(), templates.main().id()); + var elapsed = Duration.ofNanos(System.nanoTime() - startedAt); + var mainOutput = + api.semaphore().tasks().getTaskOutputText(templates.main().projectId(), completedTask.id()); + + var verificationTask = + api.semaphore() + .tasks() + .startAndWait( + templates.verifier().projectId(), + fixtures.verificationRequest(templates.verifier().id(), completedTask.id())); + var verificationOutput = + api.semaphore() + .tasks() + .getTaskOutputText(templates.verifier().projectId(), verificationTask.id()); + + assertThat(completedTask.status()).isEqualTo(fixtures.expectations().successfulTaskStatus()); + assertThat(elapsed).isLessThan(fixtures.expectations().maximumCompletionTime()); + assertThat(mainOutput) + .contains(fixtures.expectations().stdoutMarker()) + .contains(fixtures.expectations().stderrMarker()); + assertThat(verificationOutput).contains(fixtures.expectations().childGoneMarker()); + } + + private CreatedTemplates createTemplates(ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + var project = api.semaphore().projects().createProject(fixtures.project()); + var key = + api.semaphore() + .accessKeys() + .create(project.id(), fixtures.accessKey().request(project.id())); + var repository = + api.semaphore() + .repositories() + .create(project.id(), fixtures.repository().request(project.id(), key.id())); + var inventory = + api.semaphore() + .inventories() + .create(project.id(), fixtures.inventory().request(project.id(), key.id())); + var main = + api.semaphore() + .templates() + .create( + project.id(), + fixtures.templates().main().request(project.id(), repository.id(), inventory.id())); + var verifier = + api.semaphore() + .templates() + .create( + project.id(), + fixtures + .templates() + .verifier() + .request(project.id(), repository.id(), inventory.id())); + return new CreatedTemplates(main, verifier); + } + + private record CreatedTemplates(Template main, Template verifier) {} +} diff --git a/test-environment/fixtures/bash/process-cleanup/README.md b/test-environment/fixtures/bash/process-cleanup/README.md new file mode 100644 index 0000000..288b7bd --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/README.md @@ -0,0 +1,42 @@ +# Process cleanup test rig + +This fixture verifies that Semaphore removes a task's descendants after the main +command exits normally. + +## Components + +1. `main.sh` + - It is the original Semaphore task script. + - Creates `/tmp/bookwright-process-cleanup/`. + - Starts `resistant-process.sh` as a child process in the background. + - Waits until the child process records its PID. + - Prints stdout and stderr markers, then exits successfully. + +2. `resistant-process.sh` + - Runs as a descendant of the original task. + - Ignores `SIGTERM` and `SIGHUP`. + - Records its PID in the task's state directory. + - Replaces itself with `sleep 120`, keeping the original task's stdout and + stderr open. + +3. `verify.sh` + - Runs later as a second Semaphore task. + - Receives the completed task ID as an argument. + - Reads the recorded PID and checks whether that process is still running. + - Kills it if it unexpectedly survived, so a failing test does not leak a + process. + - Prints whether the process was `gone`, `alive`, or `missing`. + +## Test flow + +`LocalProcessCleanupTest` creates two Semaphore templates: one for the original +task and one for the verifier. + +1. The test starts the original task. +2. The original command exits while its resistant descendant is still running. +3. Semaphore should kill the remaining process group. +4. The test starts the verifier task with the original task ID. +5. The verifier must print `semaphore-process-cleanup-child-gone`. + +Both tasks run in the same local Semaphore container, so they can exchange the +PID through `/tmp`. The task ID gives them a shared, unique directory name. diff --git a/test-environment/fixtures/bash/process-cleanup/main.sh b/test-environment/fixtures/bash/process-cleanup/main.sh new file mode 100644 index 0000000..d8bbba4 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/main.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +state_dir="/tmp/bookwright-process-cleanup/${SEMAPHORE_TASK_DETAILS_ID}" + +rm -rf -- "$state_dir" +mkdir -p -- "$state_dir" + +bash "$script_dir/resistant-process.sh" "$state_dir" & +child_pid=$! + +for _ in $(seq 1 100); do + if [[ -s "$state_dir/process" ]]; then + break + fi + if ! kill -0 "$child_pid" 2>/dev/null; then + echo "Background child exited before recording its identity" >&2 + exit 1 + fi + sleep 0.01 +done + +if [[ ! -s "$state_dir/process" ]]; then + echo "Background child did not record its identity" >&2 + exit 1 +fi + +# Keep writes short and newline-free so output may remain buffered until EOF. +printf 'semaphore-process-cleanup-stdout-marker' +printf 'semaphore-process-cleanup-stderr-marker' >&2 diff --git a/test-environment/fixtures/bash/process-cleanup/resistant-process.sh b/test-environment/fixtures/bash/process-cleanup/resistant-process.sh new file mode 100644 index 0000000..fdf3e39 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/resistant-process.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -euo pipefail + +state_dir=$1 + +# Ignore graceful termination so only process-group cleanup can remove this process. +# +# https://man7.org/linux/man-pages/man7/signal.7.html +# > ... the dispositions of ignored signals are left unchanged. +# https://pubs.opengroup.org/onlinepubs/9799919799/functions/exec.html +# > Signals set to SIG_IGN remain ignored, while signals with caught handlers are reset to their default action. +trap '' TERM HUP + +# Publish atomically so the verifier cannot read a partially written PID. +printf '%s\n' "$$" > "$state_dir/process.tmp" +mv -- "$state_dir/process.tmp" "$state_dir/process" + +# Keep the task's inherited stdout and stderr open after its main process exits. +exec sleep 120 diff --git a/test-environment/fixtures/bash/process-cleanup/verify.sh b/test-environment/fixtures/bash/process-cleanup/verify.sh new file mode 100644 index 0000000..d4d511d --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/verify.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +set -euo pipefail + +completed_task_id=$1 +state_dir="/tmp/bookwright-process-cleanup/${completed_task_id}" +process_file="$state_dir/process" +result=missing + +# file exists and not empty +if [[ -s "$process_file" ]]; then + read -r pid < "$process_file" + state=$(awk '{print $3}' "/proc/$pid/stat" 2>/dev/null || true) + + if [[ -n "$state" && "$state" != "Z" ]]; then + result=alive + # Always clean up a surviving fixture before reporting the regression. + kill -KILL "$pid" 2>/dev/null || true + else + result=gone + fi +fi + +rm -rf -- "$state_dir" +printf 'semaphore-process-cleanup-child-%s\n' "$result" From 6178345f68df38507192f4952c68d0be147bd984 Mon Sep 17 00:00:00 2001 From: Rustam Zaitov Date: Thu, 10 Sep 2026 22:00:22 +0200 Subject: [PATCH 2/4] test: cover local process-group shutdown behavior Add API scenarios for graceful SIGTERM handling, SIGKILL escalation after the grace period, and descendants that escape into a new process group. Isolate each process topology in its own fixture directory and use follow-up verifier tasks to confirm process state and clean up intentional or unexpected survivors. Assert task status, signal delivery, and shutdown timing where applicable. --- .../SemaphoreProcessCleanupFixtures.java | 86 ++++++++++-- .../semaphore/LocalProcessCleanupTest.java | 127 +++++++++++++++--- .../fixtures/bash/process-cleanup/README.md | 42 ------ .../escaped-process-group/README.md | 30 +++++ .../escaped-process-group/child.sh | 8 ++ .../escaped-process-group/main.sh | 20 +++ .../escaped-process-group/verify.sh | 15 +++ .../process-cleanup/graceful-stop/README.md | 36 +++++ .../process-cleanup/graceful-stop/child.sh | 18 +++ .../process-cleanup/graceful-stop/main.sh | 36 +++++ .../process-cleanup/graceful-stop/verify.sh | 16 +++ .../normal-completion/README.md | 25 ++++ .../{ => normal-completion}/main.sh | 16 +-- .../resistant-process.sh | 4 +- .../normal-completion/verify.sh | 15 +++ .../process-cleanup/resistant-stop/README.md | 36 +++++ .../process-cleanup/resistant-stop/child.sh | 19 +++ .../process-cleanup/resistant-stop/main.sh | 31 +++++ .../process-cleanup/resistant-stop/verify.sh | 17 +++ .../fixtures/bash/process-cleanup/verify.sh | 24 ---- 20 files changed, 510 insertions(+), 111 deletions(-) delete mode 100644 test-environment/fixtures/bash/process-cleanup/README.md create mode 100644 test-environment/fixtures/bash/process-cleanup/escaped-process-group/README.md create mode 100644 test-environment/fixtures/bash/process-cleanup/escaped-process-group/child.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/escaped-process-group/main.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/escaped-process-group/verify.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/graceful-stop/README.md create mode 100644 test-environment/fixtures/bash/process-cleanup/graceful-stop/child.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/graceful-stop/main.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/normal-completion/README.md rename test-environment/fixtures/bash/process-cleanup/{ => normal-completion}/main.sh (61%) rename test-environment/fixtures/bash/process-cleanup/{ => normal-completion}/resistant-process.sh (76%) create mode 100644 test-environment/fixtures/bash/process-cleanup/normal-completion/verify.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/resistant-stop/README.md create mode 100644 test-environment/fixtures/bash/process-cleanup/resistant-stop/child.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/resistant-stop/main.sh create mode 100644 test-environment/fixtures/bash/process-cleanup/resistant-stop/verify.sh delete mode 100644 test-environment/fixtures/bash/process-cleanup/verify.sh diff --git a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java index dcebffd..8ea8238 100644 --- a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java +++ b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java @@ -34,19 +34,61 @@ public static SemaphoreProcessCleanupFixtures from(MainConfig config, TestData d "[local]\nlocalhost ansible_connection=local", "static"), new Templates( - new Template( - "bookwright-process-cleanup-main-" + suffix, - "test-environment/fixtures/bash/process-cleanup/main.sh", - false), - new Template( - "bookwright-process-cleanup-verifier-" + suffix, - "test-environment/fixtures/bash/process-cleanup/verify.sh", - true)), + new Scenario( + new Template( + "bookwright-process-cleanup-normal-main-" + suffix, + "test-environment/fixtures/bash/process-cleanup/normal-completion/main.sh", + false), + new Template( + "bookwright-process-cleanup-normal-verifier-" + suffix, + "test-environment/fixtures/bash/process-cleanup/normal-completion/verify.sh", + true)), + new Scenario( + new Template( + "bookwright-process-cleanup-graceful-main-" + suffix, + "test-environment/fixtures/bash/process-cleanup/graceful-stop/main.sh", + false), + new Template( + "bookwright-process-cleanup-graceful-verifier-" + suffix, + "test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh", + true)), + new Scenario( + new Template( + "bookwright-process-cleanup-resistant-main-" + suffix, + "test-environment/fixtures/bash/process-cleanup/resistant-stop/main.sh", + false), + new Template( + "bookwright-process-cleanup-resistant-verifier-" + suffix, + "test-environment/fixtures/bash/process-cleanup/resistant-stop/verify.sh", + true)), + new Scenario( + new Template( + "bookwright-process-cleanup-escaped-main-" + suffix, + "test-environment/fixtures/bash/process-cleanup/escaped-process-group/main.sh", + false), + new Template( + "bookwright-process-cleanup-escaped-verifier-" + suffix, + "test-environment/fixtures/bash/process-cleanup/escaped-process-group/verify.sh", + true))), new Expectations( "success", + "stopped", "semaphore-process-cleanup-stdout-marker", "semaphore-process-cleanup-stderr-marker", - "semaphore-process-cleanup-child-gone", + "semaphore-resistant-process-gone", + "semaphore-graceful-stop-descendants-gone", + "semaphore-process-cleanup-term-ready", + "semaphore-process-cleanup-main-term", + "semaphore-process-cleanup-child-term", + "semaphore-resistant-stop-ready", + "semaphore-resistant-stop-main-term", + "semaphore-resistant-stop-child-term", + "semaphore-resistant-stop-processes-gone", + "semaphore-escaped-process-ready", + "semaphore-escaped-process-alive", + Duration.ofSeconds(30), + Duration.ofSeconds(12), + Duration.ofSeconds(12), Duration.ofSeconds(30))); } @@ -93,12 +135,32 @@ public TemplateRequest request(long projectId, long repositoryId, long inventory } } - public record Templates(Template main, Template verifier) {} + public record Templates( + Scenario normalCompletion, + Scenario gracefulStop, + Scenario resistantStop, + Scenario escapedProcessGroup) {} + + public record Scenario(Template main, Template verifier) {} public record Expectations( String successfulTaskStatus, + String stoppedTaskStatus, String stdoutMarker, String stderrMarker, - String childGoneMarker, - Duration maximumCompletionTime) {} + String resistantProcessGoneMarker, + String gracefulDescendantsGoneMarker, + String termReadyMarker, + String mainTermMarker, + String childTermMarker, + String resistantStopReadyMarker, + String resistantStopMainTermMarker, + String resistantStopChildTermMarker, + String resistantStopProcessesGoneMarker, + String escapedProcessReadyMarker, + String escapedProcessAliveMarker, + Duration maximumCompletionTime, + Duration maximumGracefulStopTime, + Duration minimumEscalationTime, + Duration maximumEscalationTime) {} } diff --git a/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java b/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java index 2d5b200..31bc8c3 100644 --- a/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java +++ b/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java @@ -26,7 +26,7 @@ class LocalProcessCleanupTest { @DisplayName("Normal task completion cleans up a background descendant") void normalCompletionCleansUpBackgroundDescendant( ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { - var templates = createTemplates(api, fixtures); + var templates = createTemplates(api, fixtures, fixtures.templates().normalCompletion()); long startedAt = System.nanoTime(); var completedTask = @@ -35,26 +35,120 @@ void normalCompletionCleansUpBackgroundDescendant( var mainOutput = api.semaphore().tasks().getTaskOutputText(templates.main().projectId(), completedTask.id()); - var verificationTask = - api.semaphore() - .tasks() - .startAndWait( - templates.verifier().projectId(), - fixtures.verificationRequest(templates.verifier().id(), completedTask.id())); - var verificationOutput = - api.semaphore() - .tasks() - .getTaskOutputText(templates.verifier().projectId(), verificationTask.id()); + var verificationOutput = verifyCleanup(api, fixtures, templates, completedTask.id()); assertThat(completedTask.status()).isEqualTo(fixtures.expectations().successfulTaskStatus()); assertThat(elapsed).isLessThan(fixtures.expectations().maximumCompletionTime()); assertThat(mainOutput) .contains(fixtures.expectations().stdoutMarker()) .contains(fixtures.expectations().stderrMarker()); - assertThat(verificationOutput).contains(fixtures.expectations().childGoneMarker()); + assertThat(verificationOutput).contains(fixtures.expectations().resistantProcessGoneMarker()); + } + + @Test + @Preconditions(Precondition.SEMAPHORE_ADMIN_SESSION) + @DisplayName("Graceful task stop sends SIGTERM to the process group") + void gracefulStopSendsTermToProcessGroup(ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + var templates = createTemplates(api, fixtures, fixtures.templates().gracefulStop()); + var task = + api.semaphore().tasks().startTask(templates.main().projectId(), templates.main().id()); + api.semaphore() + .tasks() + .waitUntilTaskOutputContains( + templates.main().projectId(), task.id(), fixtures.expectations().termReadyMarker()); + + long stoppedAt = System.nanoTime(); + var stoppedTask = + api.semaphore().tasks().stopAndWait(templates.main().projectId(), task.id(), false); + var stopElapsed = Duration.ofNanos(System.nanoTime() - stoppedAt); + var taskOutput = + api.semaphore().tasks().getTaskOutputText(templates.main().projectId(), stoppedTask.id()); + var verificationOutput = verifyCleanup(api, fixtures, templates, stoppedTask.id()); + + assertThat(stoppedTask.status()).isEqualTo(fixtures.expectations().stoppedTaskStatus()); + assertThat(stopElapsed).isLessThan(fixtures.expectations().maximumGracefulStopTime()); + assertThat(taskOutput) + .contains(fixtures.expectations().mainTermMarker()) + .contains(fixtures.expectations().childTermMarker()); + assertThat(verificationOutput) + .contains(fixtures.expectations().gracefulDescendantsGoneMarker()); + } + + @Test + @Preconditions(Precondition.SEMAPHORE_ADMIN_SESSION) + @DisplayName("Resistant process group is killed after the grace period") + void resistantProcessGroupIsKilledAfterGracePeriod( + ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + var templates = createTemplates(api, fixtures, fixtures.templates().resistantStop()); + var task = + api.semaphore().tasks().startTask(templates.main().projectId(), templates.main().id()); + api.semaphore() + .tasks() + .waitUntilTaskOutputContains( + templates.main().projectId(), + task.id(), + fixtures.expectations().resistantStopReadyMarker()); + + long stoppedAt = System.nanoTime(); + var stoppedTask = + api.semaphore().tasks().stopAndWait(templates.main().projectId(), task.id(), false); + var stopElapsed = Duration.ofNanos(System.nanoTime() - stoppedAt); + var taskOutput = + api.semaphore().tasks().getTaskOutputText(templates.main().projectId(), stoppedTask.id()); + var verificationOutput = verifyCleanup(api, fixtures, templates, stoppedTask.id()); + + assertThat(stoppedTask.status()).isEqualTo(fixtures.expectations().stoppedTaskStatus()); + assertThat(stopElapsed) + .isGreaterThanOrEqualTo(fixtures.expectations().minimumEscalationTime()) + .isLessThan(fixtures.expectations().maximumEscalationTime()); + assertThat(taskOutput) + .contains(fixtures.expectations().resistantStopMainTermMarker()) + .contains(fixtures.expectations().resistantStopChildTermMarker()); + assertThat(verificationOutput) + .contains(fixtures.expectations().resistantStopProcessesGoneMarker()); + } + + @Test + @Preconditions(Precondition.SEMAPHORE_ADMIN_SESSION) + @DisplayName("Descendant can escape cleanup by changing its process group") + void descendantCanEscapeCleanupByChangingProcessGroup( + ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + var templates = createTemplates(api, fixtures, fixtures.templates().escapedProcessGroup()); + + long startedAt = System.nanoTime(); + var completedTask = + api.semaphore().tasks().startAndWait(templates.main().projectId(), templates.main().id()); + var elapsed = Duration.ofNanos(System.nanoTime() - startedAt); + var taskOutput = + api.semaphore().tasks().getTaskOutputText(templates.main().projectId(), completedTask.id()); + var verificationOutput = verifyCleanup(api, fixtures, templates, completedTask.id()); + + assertThat(completedTask.status()).isEqualTo(fixtures.expectations().successfulTaskStatus()); + assertThat(elapsed).isLessThan(fixtures.expectations().maximumCompletionTime()); + assertThat(taskOutput).contains(fixtures.expectations().escapedProcessReadyMarker()); + assertThat(verificationOutput).contains(fixtures.expectations().escapedProcessAliveMarker()); + } + + private String verifyCleanup( + ApiSteps api, + SemaphoreProcessCleanupFixtures fixtures, + CreatedTemplates templates, + long completedTaskId) { + var verificationTask = + api.semaphore() + .tasks() + .startAndWait( + templates.verifier().projectId(), + fixtures.verificationRequest(templates.verifier().id(), completedTaskId)); + return api.semaphore() + .tasks() + .getTaskOutputText(templates.verifier().projectId(), verificationTask.id()); } - private CreatedTemplates createTemplates(ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + private CreatedTemplates createTemplates( + ApiSteps api, + SemaphoreProcessCleanupFixtures fixtures, + SemaphoreProcessCleanupFixtures.Scenario scenario) { var project = api.semaphore().projects().createProject(fixtures.project()); var key = api.semaphore() @@ -73,16 +167,13 @@ private CreatedTemplates createTemplates(ApiSteps api, SemaphoreProcessCleanupFi .templates() .create( project.id(), - fixtures.templates().main().request(project.id(), repository.id(), inventory.id())); + scenario.main().request(project.id(), repository.id(), inventory.id())); var verifier = api.semaphore() .templates() .create( project.id(), - fixtures - .templates() - .verifier() - .request(project.id(), repository.id(), inventory.id())); + scenario.verifier().request(project.id(), repository.id(), inventory.id())); return new CreatedTemplates(main, verifier); } diff --git a/test-environment/fixtures/bash/process-cleanup/README.md b/test-environment/fixtures/bash/process-cleanup/README.md deleted file mode 100644 index 288b7bd..0000000 --- a/test-environment/fixtures/bash/process-cleanup/README.md +++ /dev/null @@ -1,42 +0,0 @@ -# Process cleanup test rig - -This fixture verifies that Semaphore removes a task's descendants after the main -command exits normally. - -## Components - -1. `main.sh` - - It is the original Semaphore task script. - - Creates `/tmp/bookwright-process-cleanup/`. - - Starts `resistant-process.sh` as a child process in the background. - - Waits until the child process records its PID. - - Prints stdout and stderr markers, then exits successfully. - -2. `resistant-process.sh` - - Runs as a descendant of the original task. - - Ignores `SIGTERM` and `SIGHUP`. - - Records its PID in the task's state directory. - - Replaces itself with `sleep 120`, keeping the original task's stdout and - stderr open. - -3. `verify.sh` - - Runs later as a second Semaphore task. - - Receives the completed task ID as an argument. - - Reads the recorded PID and checks whether that process is still running. - - Kills it if it unexpectedly survived, so a failing test does not leak a - process. - - Prints whether the process was `gone`, `alive`, or `missing`. - -## Test flow - -`LocalProcessCleanupTest` creates two Semaphore templates: one for the original -task and one for the verifier. - -1. The test starts the original task. -2. The original command exits while its resistant descendant is still running. -3. Semaphore should kill the remaining process group. -4. The test starts the verifier task with the original task ID. -5. The verifier must print `semaphore-process-cleanup-child-gone`. - -Both tasks run in the same local Semaphore container, so they can exchange the -PID through `/tmp`. The task ID gives them a shared, unique directory name. diff --git a/test-environment/fixtures/bash/process-cleanup/escaped-process-group/README.md b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/README.md new file mode 100644 index 0000000..060f0ad --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/README.md @@ -0,0 +1,30 @@ +# Escaped process group + +This fixture documents that a descendant can escape task cleanup by joining a +new session and process group. + +## Components + +1. `main.sh` starts `child.sh` through `setsid`, waits for its PID, prints a + readiness marker, and exits successfully. +2. `child.sh` ignores `SIGTERM` and `SIGHUP`, records its PID, and becomes a + long-running `sleep` process. +3. `verify.sh` confirms that the escaped process survived, kills it, and reports + `semaphore-escaped-process-alive`. + +The process tree and groups are: + +```text +main.sh [task process group] +└── child.sh → sleep 120 [new process group created by setsid] +``` + +`child.sh` uses `exec`, so `sleep` replaces it and keeps the same PID. + +## Flow + +1. The test starts the template that runs `main.sh`. +2. `setsid` moves the child into a new process group. +3. The main task exits and Semaphore cleans its process group. +4. The escaped process remains alive because it belongs to another group. +5. The verifier reports `semaphore-escaped-process-alive` and kills it. diff --git a/test-environment/fixtures/bash/process-cleanup/escaped-process-group/child.sh b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/child.sh new file mode 100644 index 0000000..ee671bd --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/child.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +set -euo pipefail + +state_dir=$1 + +trap '' TERM HUP +printf '%s\n' "$$" > "$state_dir/child.pid" +exec sleep 120 diff --git a/test-environment/fixtures/bash/process-cleanup/escaped-process-group/main.sh b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/main.sh new file mode 100644 index 0000000..dbee0bc --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/main.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +state_dir="/tmp/bookwright-process-cleanup/${SEMAPHORE_TASK_DETAILS_ID}" + +mkdir -p -- "$state_dir" +setsid bash "$script_dir/child.sh" "$state_dir" & + +for _ in {1..10}; do + [[ -s "$state_dir/child.pid" ]] && break + sleep 0.1 +done + +if [[ ! -s "$state_dir/child.pid" ]]; then + echo "Child did not become ready" >&2 + exit 1 +fi + +printf 'semaphore-escaped-process-ready\n' diff --git a/test-environment/fixtures/bash/process-cleanup/escaped-process-group/verify.sh b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/verify.sh new file mode 100644 index 0000000..d12a3fc --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/escaped-process-group/verify.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -euo pipefail + +completed_task_id=$1 +state_dir="/tmp/bookwright-process-cleanup/${completed_task_id}" +pid=$(< "$state_dir/child.pid") +result=gone + +if kill -0 "$pid" 2>/dev/null; then + result=alive + kill -KILL "$pid" +fi + +rm -rf -- "$state_dir" +printf 'semaphore-escaped-process-%s\n' "$result" diff --git a/test-environment/fixtures/bash/process-cleanup/graceful-stop/README.md b/test-environment/fixtures/bash/process-cleanup/graceful-stop/README.md new file mode 100644 index 0000000..2d638e7 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/graceful-stop/README.md @@ -0,0 +1,36 @@ +# Graceful stop cleanup + +This fixture verifies that a graceful API stop sends `SIGTERM` to the main task +process and its child. + +## Components + +1. `main.sh` starts `child.sh`, prints a readiness marker, and waits for the + child process to exit. Its TERM handler prints a marker before exiting. +2. `child.sh` records its process IDs. Its TERM handler records and prints + a marker before exiting. +3. `verify.sh` confirms that no recorded process survived and kills any + unexpected survivor. + +The running process tree is: + +```text +main.sh +└── child.sh + └── sleep 120 +``` + +## Flow + +1. The test starts the template that runs `main.sh`. +2. It waits for `semaphore-process-cleanup-term-ready`. +3. It calls the stop API with `force=false`. +4. The main and child processes handle `SIGTERM` and exit. +5. The task must reach `stopped` before the 15-second grace period expires. +6. The test confirms both signal markers, and the verifier reports + `semaphore-graceful-stop-descendants-gone`. + +The main and verifier tasks run in the same local Semaphore container. The main +task stores the process IDs in +`/tmp/bookwright-process-cleanup//children.pids`, and the Java test +passes that task ID to the verifier task. diff --git a/test-environment/fixtures/bash/process-cleanup/graceful-stop/child.sh b/test-environment/fixtures/bash/process-cleanup/graceful-stop/child.sh new file mode 100644 index 0000000..23ce9ec --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/graceful-stop/child.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +set -euo pipefail + +state_dir=$1 + +handle_term() { + printf 'semaphore-process-cleanup-child-term\n' + : > "$state_dir/child-term" + exit 0 +} +trap handle_term TERM + +sleep 120 & +sleep_pid=$! + +printf '%s\n%s\n' "$$" "$sleep_pid" > "$state_dir/children.pids" + +wait "$sleep_pid" diff --git a/test-environment/fixtures/bash/process-cleanup/graceful-stop/main.sh b/test-environment/fixtures/bash/process-cleanup/graceful-stop/main.sh new file mode 100644 index 0000000..eccf757 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/graceful-stop/main.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +state_dir="/tmp/bookwright-process-cleanup/${SEMAPHORE_TASK_DETAILS_ID}" + +mkdir -p -- "$state_dir" + +handle_term() { + printf 'semaphore-process-cleanup-main-term\n' + + # Give the child time to finish its TERM handler before the main process exits. + for _ in {1..10}; do + [[ -e "$state_dir/child-term" ]] && break + sleep 0.1 + done + # Preserve the conventional exit status for termination by SIGTERM. + exit 143 +} +trap handle_term TERM + +bash "$script_dir/child.sh" "$state_dir" & +child_pid=$! + +for _ in {1..10}; do + [[ -s "$state_dir/children.pids" ]] && break + sleep 0.1 +done + +if [[ ! -s "$state_dir/children.pids" ]]; then + echo "Child did not record its PID" >&2 + exit 1 +fi + +printf 'semaphore-process-cleanup-term-ready\n' +wait "$child_pid" diff --git a/test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh b/test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh new file mode 100644 index 0000000..005e0ce --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +set -euo pipefail + +completed_task_id=$1 +state_dir="/tmp/bookwright-process-cleanup/${completed_task_id}" +result=gone + +while read -r pid; do + if kill -0 "$pid" 2>/dev/null; then + result=alive + kill -KILL "$pid" + fi +done < "$state_dir/children.pids" + +rm -rf -- "$state_dir" +printf 'semaphore-graceful-stop-descendants-%s\n' "$result" diff --git a/test-environment/fixtures/bash/process-cleanup/normal-completion/README.md b/test-environment/fixtures/bash/process-cleanup/normal-completion/README.md new file mode 100644 index 0000000..613a66d --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/normal-completion/README.md @@ -0,0 +1,25 @@ +# Normal completion cleanup + +This fixture verifies that Semaphore removes a task's descendants after its main +command exits normally. + +## Components + +1. `main.sh` starts `resistant-process.sh` as a background child, waits for it to + record its PID, prints output markers, and exits successfully. +2. `resistant-process.sh` ignores `SIGTERM` and `SIGHUP`, then becomes a + long-running `sleep` process that keeps stdout and stderr open. +3. `verify.sh` checks that the recorded process is gone and kills it if it + unexpectedly survived. + +## Flow + +1. The test starts the template that runs `main.sh`. +2. The main command exits while its resistant descendant is still running. +3. Semaphore should kill the remaining process group. +4. The verifier must report `semaphore-resistant-process-gone`. + +The main and verifier tasks run in the same local Semaphore container. The main +task stores the PID in +`/tmp/bookwright-process-cleanup//child.pid`, and the Java test passes +that task ID to the verifier task. diff --git a/test-environment/fixtures/bash/process-cleanup/main.sh b/test-environment/fixtures/bash/process-cleanup/normal-completion/main.sh similarity index 61% rename from test-environment/fixtures/bash/process-cleanup/main.sh rename to test-environment/fixtures/bash/process-cleanup/normal-completion/main.sh index d8bbba4..9a45143 100644 --- a/test-environment/fixtures/bash/process-cleanup/main.sh +++ b/test-environment/fixtures/bash/process-cleanup/normal-completion/main.sh @@ -4,24 +4,16 @@ set -euo pipefail script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) state_dir="/tmp/bookwright-process-cleanup/${SEMAPHORE_TASK_DETAILS_ID}" -rm -rf -- "$state_dir" mkdir -p -- "$state_dir" bash "$script_dir/resistant-process.sh" "$state_dir" & -child_pid=$! -for _ in $(seq 1 100); do - if [[ -s "$state_dir/process" ]]; then - break - fi - if ! kill -0 "$child_pid" 2>/dev/null; then - echo "Background child exited before recording its identity" >&2 - exit 1 - fi - sleep 0.01 +for _ in {1..10}; do + [[ -s "$state_dir/child.pid" ]] && break + sleep 0.1 done -if [[ ! -s "$state_dir/process" ]]; then +if [[ ! -s "$state_dir/child.pid" ]]; then echo "Background child did not record its identity" >&2 exit 1 fi diff --git a/test-environment/fixtures/bash/process-cleanup/resistant-process.sh b/test-environment/fixtures/bash/process-cleanup/normal-completion/resistant-process.sh similarity index 76% rename from test-environment/fixtures/bash/process-cleanup/resistant-process.sh rename to test-environment/fixtures/bash/process-cleanup/normal-completion/resistant-process.sh index fdf3e39..41985b8 100644 --- a/test-environment/fixtures/bash/process-cleanup/resistant-process.sh +++ b/test-environment/fixtures/bash/process-cleanup/normal-completion/resistant-process.sh @@ -11,9 +11,7 @@ state_dir=$1 # > Signals set to SIG_IGN remain ignored, while signals with caught handlers are reset to their default action. trap '' TERM HUP -# Publish atomically so the verifier cannot read a partially written PID. -printf '%s\n' "$$" > "$state_dir/process.tmp" -mv -- "$state_dir/process.tmp" "$state_dir/process" +printf '%s\n' "$$" > "$state_dir/child.pid" # Keep the task's inherited stdout and stderr open after its main process exits. exec sleep 120 diff --git a/test-environment/fixtures/bash/process-cleanup/normal-completion/verify.sh b/test-environment/fixtures/bash/process-cleanup/normal-completion/verify.sh new file mode 100644 index 0000000..f694142 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/normal-completion/verify.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -euo pipefail + +completed_task_id=$1 +state_dir="/tmp/bookwright-process-cleanup/${completed_task_id}" +pid=$(< "$state_dir/child.pid") +result=gone + +if kill -0 "$pid" 2>/dev/null; then + result=alive + kill -KILL "$pid" +fi + +rm -rf -- "$state_dir" +printf 'semaphore-resistant-process-%s\n' "$result" diff --git a/test-environment/fixtures/bash/process-cleanup/resistant-stop/README.md b/test-environment/fixtures/bash/process-cleanup/resistant-stop/README.md new file mode 100644 index 0000000..dc9cc2b --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/resistant-stop/README.md @@ -0,0 +1,36 @@ +# Resistant stop cleanup + +This fixture verifies that Semaphore kills a process group that does not exit +after receiving `SIGTERM`. + +## Components + +1. `main.sh` starts `child.sh`, prints a readiness marker, and waits for the + child process. Its TERM handler prints a marker without exiting. +2. `child.sh` runs a waiting `sleep` process. Its TERM handler prints a marker + without exiting. +3. `verify.sh` confirms that the recorded processes are gone and kills any + unexpected survivor. + +The running process tree is: + +```text +main.sh +└── child.sh + └── sleep 120 +``` + +## Flow + +1. The test starts the template that runs `main.sh`. +2. It waits for `semaphore-resistant-stop-ready`. +3. It calls the stop API with `force=false`. +4. The main and child processes print their `SIGTERM` markers but remain running. +5. Semaphore waits for the 15-second grace period, then sends `SIGKILL`. +6. The task reaches `stopped` and the verifier reports + `semaphore-resistant-stop-processes-gone`. + +The main and verifier tasks run in the same local Semaphore container. The main +task stores process IDs and signal markers under +`/tmp/bookwright-process-cleanup/`, and the Java test passes that task +ID to the verifier task. diff --git a/test-environment/fixtures/bash/process-cleanup/resistant-stop/child.sh b/test-environment/fixtures/bash/process-cleanup/resistant-stop/child.sh new file mode 100644 index 0000000..d6a38e4 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/resistant-stop/child.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -euo pipefail + +state_dir=$1 + +handle_term() { + printf 'semaphore-resistant-stop-child-term\n' +} +trap handle_term TERM + +printf '%s\n' "$$" > "$state_dir/child.pid" + +# Keep this script alive by starting a new sleep after SIGTERM stops the current one. +while true; do + sleep 120 & + sleep_pid=$! + printf '%s\n' "$sleep_pid" > "$state_dir/sleep.pid" + wait "$sleep_pid" || true +done diff --git a/test-environment/fixtures/bash/process-cleanup/resistant-stop/main.sh b/test-environment/fixtures/bash/process-cleanup/resistant-stop/main.sh new file mode 100644 index 0000000..073a106 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/resistant-stop/main.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +state_dir="/tmp/bookwright-process-cleanup/${SEMAPHORE_TASK_DETAILS_ID}" + +mkdir -p -- "$state_dir" +printf '%s\n' "$$" > "$state_dir/main.pid" + +handle_term() { + printf 'semaphore-resistant-stop-main-term\n' +} +trap handle_term TERM + +bash "$script_dir/child.sh" "$state_dir" & +child_pid=$! + +for _ in {1..10}; do + [[ -s "$state_dir/sleep.pid" ]] && break + sleep 0.1 +done + +if [[ ! -s "$state_dir/sleep.pid" ]]; then + echo "Child did not become ready" >&2 + exit 1 +fi + +printf 'semaphore-resistant-stop-ready\n' +while true; do + wait "$child_pid" || true +done diff --git a/test-environment/fixtures/bash/process-cleanup/resistant-stop/verify.sh b/test-environment/fixtures/bash/process-cleanup/resistant-stop/verify.sh new file mode 100644 index 0000000..8c23b5a --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/resistant-stop/verify.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -euo pipefail + +completed_task_id=$1 +state_dir="/tmp/bookwright-process-cleanup/${completed_task_id}" +result=gone + +for pid_file in main.pid child.pid sleep.pid; do + pid=$(< "$state_dir/$pid_file") + if kill -0 "$pid" 2>/dev/null; then + result=alive + kill -KILL "$pid" + fi +done + +rm -rf -- "$state_dir" +printf 'semaphore-resistant-stop-processes-%s\n' "$result" diff --git a/test-environment/fixtures/bash/process-cleanup/verify.sh b/test-environment/fixtures/bash/process-cleanup/verify.sh deleted file mode 100644 index d4d511d..0000000 --- a/test-environment/fixtures/bash/process-cleanup/verify.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -completed_task_id=$1 -state_dir="/tmp/bookwright-process-cleanup/${completed_task_id}" -process_file="$state_dir/process" -result=missing - -# file exists and not empty -if [[ -s "$process_file" ]]; then - read -r pid < "$process_file" - state=$(awk '{print $3}' "/proc/$pid/stat" 2>/dev/null || true) - - if [[ -n "$state" && "$state" != "Z" ]]; then - result=alive - # Always clean up a surviving fixture before reporting the regression. - kill -KILL "$pid" 2>/dev/null || true - else - result=gone - fi -fi - -rm -rf -- "$state_dir" -printf 'semaphore-process-cleanup-child-%s\n' "$result" From 32afbf5d3197d1f60efe1304deeb8c34dcd30b53 Mon Sep 17 00:00:00 2001 From: Rustam Zaitov Date: Tue, 15 Sep 2026 17:23:42 +0200 Subject: [PATCH 3/4] test: cover graceful cancellation with zero exit Verify that a task remains stopped when its command handles SIGTERM and exits successfully, preventing cancellation from being interpreted as normal completion. --- .../SemaphoreProcessCleanupFixtures.java | 9 ++++ .../semaphore/LocalProcessCleanupTest.java | 42 +++++++++++++++++++ .../graceful-zero-exit/README.md | 24 +++++++++++ .../graceful-zero-exit/main.sh | 13 ++++++ 4 files changed, 88 insertions(+) create mode 100644 test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/README.md create mode 100644 test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/main.sh diff --git a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java index 8ea8238..55fa4dd 100644 --- a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java +++ b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java @@ -52,6 +52,10 @@ public static SemaphoreProcessCleanupFixtures from(MainConfig config, TestData d "bookwright-process-cleanup-graceful-verifier-" + suffix, "test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh", true)), + new Template( + "bookwright-process-cleanup-graceful-zero-exit-" + suffix, + "test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/main.sh", + false), new Scenario( new Template( "bookwright-process-cleanup-resistant-main-" + suffix, @@ -80,6 +84,8 @@ public static SemaphoreProcessCleanupFixtures from(MainConfig config, TestData d "semaphore-process-cleanup-term-ready", "semaphore-process-cleanup-main-term", "semaphore-process-cleanup-child-term", + "semaphore-graceful-zero-exit-ready", + "semaphore-graceful-zero-exit-term", "semaphore-resistant-stop-ready", "semaphore-resistant-stop-main-term", "semaphore-resistant-stop-child-term", @@ -138,6 +144,7 @@ public TemplateRequest request(long projectId, long repositoryId, long inventory public record Templates( Scenario normalCompletion, Scenario gracefulStop, + Template gracefulZeroExit, Scenario resistantStop, Scenario escapedProcessGroup) {} @@ -153,6 +160,8 @@ public record Expectations( String termReadyMarker, String mainTermMarker, String childTermMarker, + String gracefulZeroExitReadyMarker, + String gracefulZeroExitTermMarker, String resistantStopReadyMarker, String resistantStopMainTermMarker, String resistantStopChildTermMarker, diff --git a/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java b/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java index 31bc8c3..95af448 100644 --- a/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java +++ b/src/test/java/io/bookwright/tests/semaphore/LocalProcessCleanupTest.java @@ -74,6 +74,26 @@ void gracefulStopSendsTermToProcessGroup(ApiSteps api, SemaphoreProcessCleanupFi .contains(fixtures.expectations().gracefulDescendantsGoneMarker()); } + @Test + @Preconditions(Precondition.SEMAPHORE_ADMIN_SESSION) + @DisplayName("Graceful task stop remains stopped when the command exits successfully") + void gracefulStopWithZeroExitRemainsStopped( + ApiSteps api, SemaphoreProcessCleanupFixtures fixtures) { + var template = createTemplate(api, fixtures, fixtures.templates().gracefulZeroExit()); + var task = api.semaphore().tasks().startTask(template.projectId(), template.id()); + api.semaphore() + .tasks() + .waitUntilTaskOutputContains( + template.projectId(), task.id(), fixtures.expectations().gracefulZeroExitReadyMarker()); + + var stoppedTask = api.semaphore().tasks().stopAndWait(template.projectId(), task.id(), false); + var taskOutput = + api.semaphore().tasks().getTaskOutputText(template.projectId(), stoppedTask.id()); + + assertThat(stoppedTask.status()).isEqualTo(fixtures.expectations().stoppedTaskStatus()); + assertThat(taskOutput).contains(fixtures.expectations().gracefulZeroExitTermMarker()); + } + @Test @Preconditions(Precondition.SEMAPHORE_ADMIN_SESSION) @DisplayName("Resistant process group is killed after the grace period") @@ -145,6 +165,28 @@ private String verifyCleanup( .getTaskOutputText(templates.verifier().projectId(), verificationTask.id()); } + private Template createTemplate( + ApiSteps api, + SemaphoreProcessCleanupFixtures fixtures, + SemaphoreProcessCleanupFixtures.Template template) { + var project = api.semaphore().projects().createProject(fixtures.project()); + var key = + api.semaphore() + .accessKeys() + .create(project.id(), fixtures.accessKey().request(project.id())); + var repository = + api.semaphore() + .repositories() + .create(project.id(), fixtures.repository().request(project.id(), key.id())); + var inventory = + api.semaphore() + .inventories() + .create(project.id(), fixtures.inventory().request(project.id(), key.id())); + return api.semaphore() + .templates() + .create(project.id(), template.request(project.id(), repository.id(), inventory.id())); + } + private CreatedTemplates createTemplates( ApiSteps api, SemaphoreProcessCleanupFixtures fixtures, diff --git a/test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/README.md b/test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/README.md new file mode 100644 index 0000000..4e698e0 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/README.md @@ -0,0 +1,24 @@ +# Graceful stop with zero exit + +This fixture verifies that a task remains cancelled when its main process handles +`SIGTERM` and exits with status `0`. + +Once `SIGTERM` is handled gracefully, both situations can produce exit code `0`: + +```text +normal completion → exit 0 +cancellation → handle SIGTERM → exit 0 +``` + +## Flow + +1. Start a Bash template that runs `main.sh`. +2. Wait until the main script is ready. +3. Request a regular stop with `force=false`. +4. The script handles `SIGTERM` and exits with status `0`. +5. The task must reach `stopped`; the clean process exit must not be interpreted + as normal task completion. + +This scenario deliberately makes cancellation look like normal completion by +exiting with status `0` after `SIGTERM`. Semaphore must still finalize the task +as `stopped`. diff --git a/test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/main.sh b/test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/main.sh new file mode 100644 index 0000000..492af47 --- /dev/null +++ b/test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/main.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +set -euo pipefail + +handle_term() { + printf 'semaphore-graceful-zero-exit-term\n' + exit 0 +} +trap handle_term TERM + +printf 'semaphore-graceful-zero-exit-ready\n' +while true; do + sleep 1 +done From ec094e7c99daeff0dcd003175d7770ffc135e8a0 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 17 Sep 2026 12:57:15 +0300 Subject: [PATCH 4/4] chore: fix fixtures path --- .../SemaphoreProcessCleanupFixtures.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java index 55fa4dd..9056c9f 100644 --- a/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java +++ b/src/main/java/io/bookwright/fixtures/semaphore/SemaphoreProcessCleanupFixtures.java @@ -37,42 +37,42 @@ public static SemaphoreProcessCleanupFixtures from(MainConfig config, TestData d new Scenario( new Template( "bookwright-process-cleanup-normal-main-" + suffix, - "test-environment/fixtures/bash/process-cleanup/normal-completion/main.sh", + "bash/process-cleanup/normal-completion/main.sh", false), new Template( "bookwright-process-cleanup-normal-verifier-" + suffix, - "test-environment/fixtures/bash/process-cleanup/normal-completion/verify.sh", + "bash/process-cleanup/normal-completion/verify.sh", true)), new Scenario( new Template( "bookwright-process-cleanup-graceful-main-" + suffix, - "test-environment/fixtures/bash/process-cleanup/graceful-stop/main.sh", + "bash/process-cleanup/graceful-stop/main.sh", false), new Template( "bookwright-process-cleanup-graceful-verifier-" + suffix, - "test-environment/fixtures/bash/process-cleanup/graceful-stop/verify.sh", + "bash/process-cleanup/graceful-stop/verify.sh", true)), new Template( "bookwright-process-cleanup-graceful-zero-exit-" + suffix, - "test-environment/fixtures/bash/process-cleanup/graceful-zero-exit/main.sh", + "bash/process-cleanup/graceful-zero-exit/main.sh", false), new Scenario( new Template( "bookwright-process-cleanup-resistant-main-" + suffix, - "test-environment/fixtures/bash/process-cleanup/resistant-stop/main.sh", + "bash/process-cleanup/resistant-stop/main.sh", false), new Template( "bookwright-process-cleanup-resistant-verifier-" + suffix, - "test-environment/fixtures/bash/process-cleanup/resistant-stop/verify.sh", + "bash/process-cleanup/resistant-stop/verify.sh", true)), new Scenario( new Template( "bookwright-process-cleanup-escaped-main-" + suffix, - "test-environment/fixtures/bash/process-cleanup/escaped-process-group/main.sh", + "bash/process-cleanup/escaped-process-group/main.sh", false), new Template( "bookwright-process-cleanup-escaped-verifier-" + suffix, - "test-environment/fixtures/bash/process-cleanup/escaped-process-group/verify.sh", + "bash/process-cleanup/escaped-process-group/verify.sh", true))), new Expectations( "success",