From 7409b1670b8d80a3872e9d434cec9ca4620d58b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Thu, 26 Feb 2026 22:37:07 -0700 Subject: [PATCH 1/3] Fix #287: terminate scheduler on abort to prevent HALT hang When abort() kills child jobs, the scheduler itself was not being terminated. This left yath hanging after a BAIL_OUT event, requiring ctrl+c to exit. Adding $self->terminate(1) after the kill loop ensures the scheduler exits its main loop once abort completes. Safe with --no-abort-on-bail: that flag gates job_update() (which never calls abort()), not the abort() method itself. Also replaces the stub Scheduler.t with real unit tests (7 subtests) covering abort, kill, terminate, and selective run abort behavior. Adds integration test for BAIL_OUT termination. Co-Authored-By: Claude Opus 4.6 --- lib/Test2/Harness/Scheduler.pm | 3 + t/integration/bailout.t | 49 +++++++++++ t/integration/bailout/bail.tx | 11 +++ t/integration/bailout/pass.tx | 7 ++ t/unit/Test2/Harness/Scheduler.t | 139 ++++++++++++++++++++++++++++++- 5 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 t/integration/bailout.t create mode 100644 t/integration/bailout/bail.tx create mode 100644 t/integration/bailout/pass.tx diff --git a/lib/Test2/Harness/Scheduler.pm b/lib/Test2/Harness/Scheduler.pm index 1a71cf031..a852fc44e 100644 --- a/lib/Test2/Harness/Scheduler.pm +++ b/lib/Test2/Harness/Scheduler.pm @@ -307,6 +307,9 @@ sub abort { CORE::kill('TERM', $pid); $job->{killed} = 1; } + + # Also terminate the scheduler itself, as BAIL_OUT means we should exit + $self->terminate(1); } sub kill { diff --git a/t/integration/bailout.t b/t/integration/bailout.t new file mode 100644 index 000000000..a63bbb17a --- /dev/null +++ b/t/integration/bailout.t @@ -0,0 +1,49 @@ +use Test2::V0; +# HARNESS-DURATION-LONG + +use App::Yath::Tester qw/yath/; +use Test2::Plugin::Immiscible(sub { $ENV{TEST2_HARNESS_ACTIVE} ? 1 : 0 }); + +my $dir = __FILE__; +$dir =~ s{\.t$}{}g; +$dir =~ s{^\./}{}; + +# Test that BAIL_OUT causes yath to exit (not hang) +# Regression test for https://github.com/Test-More/Test2-Harness/issues/287 +yath( + command => 'test', + args => [$dir, '--ext=tx'], + exit => T(), + test => sub { + my $out = shift; + ok($out->{exit}, "yath exits with non-zero when BAIL_OUT is encountered"); + like($out->{output}, qr/BAIL_OUT|bail|halt/i, "output mentions bail/halt"); + }, +); + +# Test that BAIL_OUT is ignored when --no-abort-on-bail is set +yath( + command => 'test', + args => [$dir, '--ext=tx', '--no-abort-on-bail'], + exit => T(), + test => sub { + my $out = shift; + # The test still fails (BAIL_OUT is a failure), but yath should + # not abort the entire suite — just the bailing test fails + ok($out->{exit}, "yath exits non-zero (test still fails)"); + }, +); + +# Test that when BAIL_OUT is disabled via env, everything passes +yath( + command => 'test', + args => [$dir, '--ext=tx'], + env => {BAILOUT_DO_PASS => 1}, + exit => 0, + test => sub { + my $out = shift; + ok(!$out->{exit}, "yath exits cleanly when BAIL_OUT is not triggered"); + }, +); + +done_testing; diff --git a/t/integration/bailout/bail.tx b/t/integration/bailout/bail.tx new file mode 100644 index 000000000..beff35fd6 --- /dev/null +++ b/t/integration/bailout/bail.tx @@ -0,0 +1,11 @@ +use Test2::Tools::Tiny; +use strict; +use warnings; + +ok(1, "a passing test before bail"); + +BAIL_OUT("Something went horribly wrong") unless $ENV{BAILOUT_DO_PASS}; + +ok(1, "should not reach here"); + +done_testing; diff --git a/t/integration/bailout/pass.tx b/t/integration/bailout/pass.tx new file mode 100644 index 000000000..fbfd38371 --- /dev/null +++ b/t/integration/bailout/pass.tx @@ -0,0 +1,7 @@ +use Test2::Tools::Tiny; +use strict; +use warnings; + +ok(1, "this test always passes"); + +done_testing; diff --git a/t/unit/Test2/Harness/Scheduler.t b/t/unit/Test2/Harness/Scheduler.t index bcaf33042..4be799752 100644 --- a/t/unit/Test2/Harness/Scheduler.t +++ b/t/unit/Test2/Harness/Scheduler.t @@ -1,5 +1,142 @@ use Test2::V0 -target => 'Test2::Harness::Scheduler'; -skip_all "write me"; +use File::Temp qw/tempdir/; +use Test2::Harness::Runner; +use Test2::Harness::TestSettings; + +my $dir = tempdir(CLEANUP => 1); + +sub make_scheduler { + my %params = @_; + + my $runner = Test2::Harness::Runner->new( + workdir => $dir, + test_settings => Test2::Harness::TestSettings->new(), + ); + + return $CLASS->new(runner => $runner, %params); +} + +subtest 'abort terminates the scheduler' => sub { + my $sched = make_scheduler(); + + ok(!$sched->terminated, "scheduler is not terminated initially"); + + $sched->abort(); + + ok($sched->terminated, "scheduler is terminated after abort()"); + is($sched->terminated, 1, "terminate reason is 1"); +}; + +subtest 'abort with no running jobs still terminates' => sub { + my $sched = make_scheduler(); + + ok(!$sched->terminated, "scheduler is not terminated initially"); + is($sched->{running}{jobs}, undef, "no running jobs"); + + $sched->abort(); + + ok($sched->terminated, "scheduler terminates even with no running jobs"); +}; + +subtest 'kill delegates to abort and terminates' => sub { + my $sched = make_scheduler(); + + ok(!$sched->terminated, "scheduler is not terminated initially"); + + $sched->kill(); + + ok($sched->terminated, "scheduler is terminated after kill()"); +}; + +subtest 'terminate is idempotent' => sub { + my $sched = make_scheduler(); + + my $reason1 = $sched->terminate('first'); + is($reason1, 'first', "first terminate returns the reason"); + + my $reason2 = $sched->terminate('second'); + is($reason2, 'first', "second terminate returns original reason"); +}; + +subtest 'abort marks runs as halted' => sub { + my $sched = make_scheduler(); + + # Create a mock run object + my $halt_value; + my $mock_run = mock {} => ( + add => [ + set_halt => sub { $halt_value = $_[1] }, + run_id => sub { 'run-1' }, + ], + ); + + $sched->{runs}{'run-1'} = $mock_run; + + $sched->abort(); + + is($halt_value, 'aborted', "run was marked as halted/aborted"); + ok($sched->terminated, "scheduler terminated after abort"); +}; + +subtest 'abort kills running job PIDs' => sub { + my $sched = make_scheduler(); + + my $halt_value; + my $mock_run = mock {} => ( + add => [ + set_halt => sub { $halt_value = $_[1] }, + run_id => sub { 'run-1' }, + ], + ); + + $sched->{runs}{'run-1'} = $mock_run; + + # Use a child process so TERM doesn't kill us + my $child = fork(); + if (!defined $child) { + skip_all "fork failed: $!"; + } + elsif ($child == 0) { + sleep 10; + exit 0; + } + + $sched->{running}{jobs}{'job-1'} = { + run => $mock_run, + pid => $child, + killed => 0, + }; + + $sched->abort(); + + ok($sched->{running}{jobs}{'job-1'}{killed}, "job was marked as killed"); + ok($sched->terminated, "scheduler terminated after abort with running jobs"); + + # Clean up child + kill('KILL', $child); + waitpid($child, 0); +}; + +subtest 'abort with specific run IDs only aborts those runs' => sub { + my $sched = make_scheduler(); + + my %halt_values; + for my $id ('run-1', 'run-2') { + my $run_id = $id; + $sched->{runs}{$id} = mock {} => ( + add => [ + set_halt => sub { $halt_values{$run_id} = $_[1] }, + run_id => sub { $run_id }, + ], + ); + } + + $sched->abort('run-1'); + + is($halt_values{'run-1'}, 'aborted', "run-1 was aborted"); + ok(!exists $halt_values{'run-2'}, "run-2 was not aborted"); + ok($sched->terminated, "scheduler still terminates"); +}; done_testing; From 2abaf1a6a2376f85f2692e144a92dff1ffcf4aa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Fri, 27 Feb 2026 16:40:51 -0700 Subject: [PATCH 2/3] rebase: apply review feedback on #303 --- Makefile.PL | 4 +++- cpanfile | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile.PL b/Makefile.PL index facf9eeb7..78d2d13ce 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -70,6 +70,7 @@ my %WriteMakefileArgs = ( "File::ShareDir" => 0, "File::Spec" => 0, "File::Temp" => 0, + "Getopt::Yath" => "2.000007", "HTTP::Tiny" => 0, "HTTP::Tiny::Multipart" => 0, "HTTP::Tiny::UNIX" => 0, @@ -149,7 +150,7 @@ my %WriteMakefileArgs = ( }, "VERSION" => "2.000007", "test" => { - "TESTS" => "t/*.t t/JUnit/*.t t/UI/*.t t/acceptence/*.t t/database/*.t t/integration/*.t t/integration/signals/*.t t/unit/App/*.t t/unit/App/Yath/*.t t/unit/App/Yath/Command/*.t t/unit/App/Yath/Command/client/*.t t/unit/App/Yath/Command/db/*.t t/unit/App/Yath/Options/*.t t/unit/App/Yath/Plugin/*.t t/unit/App/Yath/Renderer/*.t t/unit/App/Yath/Renderer/Default/*.t t/unit/App/Yath/Resource/*.t t/unit/App/Yath/Resource/SharedJobSlots/*.t t/unit/App/Yath/Schema/*.t t/unit/App/Yath/Schema/MariaDB/*.t t/unit/App/Yath/Schema/MySQL/*.t t/unit/App/Yath/Schema/Overlay/*.t t/unit/App/Yath/Schema/Percona/*.t t/unit/App/Yath/Schema/PostgreSQL/*.t t/unit/App/Yath/Schema/Result/*.t t/unit/App/Yath/Schema/SQLite/*.t t/unit/App/Yath/Server/*.t t/unit/App/Yath/Server/Controller/*.t t/unit/App/Yath/Server/Util/*.t t/unit/App/Yath/Theme/*.t t/unit/Getopt/Yath/*.t t/unit/Getopt/Yath/Option/*.t t/unit/Getopt/Yath/Settings/*.t t/unit/TAP/Harness/*.t t/unit/TAP/Harness/Yath/*.t t/unit/Test2/*.t t/unit/Test2/EventFacet/*.t t/unit/Test2/Formatter/*.t t/unit/Test2/Harness/*.t t/unit/Test2/Harness/Collector/*.t t/unit/Test2/Harness/Collector/Auditor/*.t t/unit/Test2/Harness/Collector/IOParser/*.t t/unit/Test2/Harness/IPC/*.t t/unit/Test2/Harness/IPC/Protocol/*.t t/unit/Test2/Harness/IPC/Protocol/AtomicPipe/*.t t/unit/Test2/Harness/IPC/Protocol/IPSocket/*.t t/unit/Test2/Harness/IPC/Protocol/UnixSocket/*.t t/unit/Test2/Harness/Instance/*.t t/unit/Test2/Harness/Log/*.t t/unit/Test2/Harness/Log/CoverageAggregator/*.t t/unit/Test2/Harness/Preload/*.t t/unit/Test2/Harness/Reloader/*.t t/unit/Test2/Harness/Resource/*.t t/unit/Test2/Harness/Run/*.t t/unit/Test2/Harness/Runner/*.t t/unit/Test2/Harness/Runner/Preloading/*.t t/unit/Test2/Harness/Scheduler/*.t t/unit/Test2/Harness/Util/*.t t/unit/Test2/Harness/Util/File/*.t t/unit/Test2/Tools/*.t" + "TESTS" => "t/*.t t/JUnit/*.t t/UI/*.t t/acceptence/*.t t/database/*.t t/integration/*.t t/integration/signals/*.t t/unit/App/*.t t/unit/App/Yath/*.t t/unit/App/Yath/Command/*.t t/unit/App/Yath/Command/client/*.t t/unit/App/Yath/Command/db/*.t t/unit/App/Yath/Options/*.t t/unit/App/Yath/Plugin/*.t t/unit/App/Yath/Renderer/*.t t/unit/App/Yath/Renderer/Default/*.t t/unit/App/Yath/Resource/*.t t/unit/App/Yath/Resource/SharedJobSlots/*.t t/unit/App/Yath/Schema/*.t t/unit/App/Yath/Schema/MariaDB/*.t t/unit/App/Yath/Schema/MySQL/*.t t/unit/App/Yath/Schema/Overlay/*.t t/unit/App/Yath/Schema/Percona/*.t t/unit/App/Yath/Schema/PostgreSQL/*.t t/unit/App/Yath/Schema/Result/*.t t/unit/App/Yath/Schema/SQLite/*.t t/unit/App/Yath/Server/*.t t/unit/App/Yath/Server/Controller/*.t t/unit/App/Yath/Server/Util/*.t t/unit/App/Yath/Theme/*.t t/unit/TAP/Harness/*.t t/unit/TAP/Harness/Yath/*.t t/unit/Test2/*.t t/unit/Test2/EventFacet/*.t t/unit/Test2/Formatter/*.t t/unit/Test2/Harness/*.t t/unit/Test2/Harness/Collector/*.t t/unit/Test2/Harness/Collector/Auditor/*.t t/unit/Test2/Harness/Collector/IOParser/*.t t/unit/Test2/Harness/IPC/*.t t/unit/Test2/Harness/IPC/Protocol/*.t t/unit/Test2/Harness/IPC/Protocol/AtomicPipe/*.t t/unit/Test2/Harness/IPC/Protocol/IPSocket/*.t t/unit/Test2/Harness/IPC/Protocol/UnixSocket/*.t t/unit/Test2/Harness/Instance/*.t t/unit/Test2/Harness/Log/*.t t/unit/Test2/Harness/Log/CoverageAggregator/*.t t/unit/Test2/Harness/Preload/*.t t/unit/Test2/Harness/Reloader/*.t t/unit/Test2/Harness/Resource/*.t t/unit/Test2/Harness/Run/*.t t/unit/Test2/Harness/Runner/*.t t/unit/Test2/Harness/Runner/Preloading/*.t t/unit/Test2/Harness/Scheduler/*.t t/unit/Test2/Harness/Util/*.t t/unit/Test2/Harness/Util/File/*.t t/unit/Test2/Tools/*.t" } ); @@ -182,6 +183,7 @@ my %FallbackPrereqs = ( "File::ShareDir" => 0, "File::Spec" => 0, "File::Temp" => 0, + "Getopt::Yath" => "2.000007", "HTTP::Tiny" => 0, "HTTP::Tiny::Multipart" => 0, "HTTP::Tiny::UNIX" => 0, diff --git a/cpanfile b/cpanfile index 606331a7c..822248f68 100644 --- a/cpanfile +++ b/cpanfile @@ -27,6 +27,7 @@ requires "File::Path" => "2.11"; requires "File::ShareDir" => "0"; requires "File::Spec" => "0"; requires "File::Temp" => "0"; +requires "Getopt::Yath" => "2.000007"; requires "HTTP::Tiny" => "0"; requires "HTTP::Tiny::Multipart" => "0"; requires "HTTP::Tiny::UNIX" => "0"; From 035c6aee60d97235fec414758849f1af66bdbbf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Fri, 27 Feb 2026 17:03:33 -0700 Subject: [PATCH 3/3] rebase: apply review feedback on #303 --- Makefile.PL | 4 +--- cpanfile | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile.PL b/Makefile.PL index 78d2d13ce..facf9eeb7 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -70,7 +70,6 @@ my %WriteMakefileArgs = ( "File::ShareDir" => 0, "File::Spec" => 0, "File::Temp" => 0, - "Getopt::Yath" => "2.000007", "HTTP::Tiny" => 0, "HTTP::Tiny::Multipart" => 0, "HTTP::Tiny::UNIX" => 0, @@ -150,7 +149,7 @@ my %WriteMakefileArgs = ( }, "VERSION" => "2.000007", "test" => { - "TESTS" => "t/*.t t/JUnit/*.t t/UI/*.t t/acceptence/*.t t/database/*.t t/integration/*.t t/integration/signals/*.t t/unit/App/*.t t/unit/App/Yath/*.t t/unit/App/Yath/Command/*.t t/unit/App/Yath/Command/client/*.t t/unit/App/Yath/Command/db/*.t t/unit/App/Yath/Options/*.t t/unit/App/Yath/Plugin/*.t t/unit/App/Yath/Renderer/*.t t/unit/App/Yath/Renderer/Default/*.t t/unit/App/Yath/Resource/*.t t/unit/App/Yath/Resource/SharedJobSlots/*.t t/unit/App/Yath/Schema/*.t t/unit/App/Yath/Schema/MariaDB/*.t t/unit/App/Yath/Schema/MySQL/*.t t/unit/App/Yath/Schema/Overlay/*.t t/unit/App/Yath/Schema/Percona/*.t t/unit/App/Yath/Schema/PostgreSQL/*.t t/unit/App/Yath/Schema/Result/*.t t/unit/App/Yath/Schema/SQLite/*.t t/unit/App/Yath/Server/*.t t/unit/App/Yath/Server/Controller/*.t t/unit/App/Yath/Server/Util/*.t t/unit/App/Yath/Theme/*.t t/unit/TAP/Harness/*.t t/unit/TAP/Harness/Yath/*.t t/unit/Test2/*.t t/unit/Test2/EventFacet/*.t t/unit/Test2/Formatter/*.t t/unit/Test2/Harness/*.t t/unit/Test2/Harness/Collector/*.t t/unit/Test2/Harness/Collector/Auditor/*.t t/unit/Test2/Harness/Collector/IOParser/*.t t/unit/Test2/Harness/IPC/*.t t/unit/Test2/Harness/IPC/Protocol/*.t t/unit/Test2/Harness/IPC/Protocol/AtomicPipe/*.t t/unit/Test2/Harness/IPC/Protocol/IPSocket/*.t t/unit/Test2/Harness/IPC/Protocol/UnixSocket/*.t t/unit/Test2/Harness/Instance/*.t t/unit/Test2/Harness/Log/*.t t/unit/Test2/Harness/Log/CoverageAggregator/*.t t/unit/Test2/Harness/Preload/*.t t/unit/Test2/Harness/Reloader/*.t t/unit/Test2/Harness/Resource/*.t t/unit/Test2/Harness/Run/*.t t/unit/Test2/Harness/Runner/*.t t/unit/Test2/Harness/Runner/Preloading/*.t t/unit/Test2/Harness/Scheduler/*.t t/unit/Test2/Harness/Util/*.t t/unit/Test2/Harness/Util/File/*.t t/unit/Test2/Tools/*.t" + "TESTS" => "t/*.t t/JUnit/*.t t/UI/*.t t/acceptence/*.t t/database/*.t t/integration/*.t t/integration/signals/*.t t/unit/App/*.t t/unit/App/Yath/*.t t/unit/App/Yath/Command/*.t t/unit/App/Yath/Command/client/*.t t/unit/App/Yath/Command/db/*.t t/unit/App/Yath/Options/*.t t/unit/App/Yath/Plugin/*.t t/unit/App/Yath/Renderer/*.t t/unit/App/Yath/Renderer/Default/*.t t/unit/App/Yath/Resource/*.t t/unit/App/Yath/Resource/SharedJobSlots/*.t t/unit/App/Yath/Schema/*.t t/unit/App/Yath/Schema/MariaDB/*.t t/unit/App/Yath/Schema/MySQL/*.t t/unit/App/Yath/Schema/Overlay/*.t t/unit/App/Yath/Schema/Percona/*.t t/unit/App/Yath/Schema/PostgreSQL/*.t t/unit/App/Yath/Schema/Result/*.t t/unit/App/Yath/Schema/SQLite/*.t t/unit/App/Yath/Server/*.t t/unit/App/Yath/Server/Controller/*.t t/unit/App/Yath/Server/Util/*.t t/unit/App/Yath/Theme/*.t t/unit/Getopt/Yath/*.t t/unit/Getopt/Yath/Option/*.t t/unit/Getopt/Yath/Settings/*.t t/unit/TAP/Harness/*.t t/unit/TAP/Harness/Yath/*.t t/unit/Test2/*.t t/unit/Test2/EventFacet/*.t t/unit/Test2/Formatter/*.t t/unit/Test2/Harness/*.t t/unit/Test2/Harness/Collector/*.t t/unit/Test2/Harness/Collector/Auditor/*.t t/unit/Test2/Harness/Collector/IOParser/*.t t/unit/Test2/Harness/IPC/*.t t/unit/Test2/Harness/IPC/Protocol/*.t t/unit/Test2/Harness/IPC/Protocol/AtomicPipe/*.t t/unit/Test2/Harness/IPC/Protocol/IPSocket/*.t t/unit/Test2/Harness/IPC/Protocol/UnixSocket/*.t t/unit/Test2/Harness/Instance/*.t t/unit/Test2/Harness/Log/*.t t/unit/Test2/Harness/Log/CoverageAggregator/*.t t/unit/Test2/Harness/Preload/*.t t/unit/Test2/Harness/Reloader/*.t t/unit/Test2/Harness/Resource/*.t t/unit/Test2/Harness/Run/*.t t/unit/Test2/Harness/Runner/*.t t/unit/Test2/Harness/Runner/Preloading/*.t t/unit/Test2/Harness/Scheduler/*.t t/unit/Test2/Harness/Util/*.t t/unit/Test2/Harness/Util/File/*.t t/unit/Test2/Tools/*.t" } ); @@ -183,7 +182,6 @@ my %FallbackPrereqs = ( "File::ShareDir" => 0, "File::Spec" => 0, "File::Temp" => 0, - "Getopt::Yath" => "2.000007", "HTTP::Tiny" => 0, "HTTP::Tiny::Multipart" => 0, "HTTP::Tiny::UNIX" => 0, diff --git a/cpanfile b/cpanfile index 822248f68..606331a7c 100644 --- a/cpanfile +++ b/cpanfile @@ -27,7 +27,6 @@ requires "File::Path" => "2.11"; requires "File::ShareDir" => "0"; requires "File::Spec" => "0"; requires "File::Temp" => "0"; -requires "Getopt::Yath" => "2.000007"; requires "HTTP::Tiny" => "0"; requires "HTTP::Tiny::Multipart" => "0"; requires "HTTP::Tiny::UNIX" => "0";