Skip to content

Commit bf85ebc

Browse files
Koan-Botclaude
andcommitted
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 stub test files with real unit tests: - Scheduler.t: 7 subtests covering abort, kill, terminate, and selective run abort behavior - Command.t: base class behavior (name derivation, run, set_dot_args, constructor) - Command/init.t: run() creates test.pl, overwrites generated files, rejects non-generated files - Command/failed.t: run() parses JSONL logs, detects failures, brief mode, input validation - Command/times.t: run() displays timing data, sorts by duration, input validation - Command/help.t: run() produces command listing output Adds integration test for BAIL_OUT termination. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 61c1fe8 commit bf85ebc

11 files changed

Lines changed: 673 additions & 7 deletions

File tree

Makefile.PL

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ my %WriteMakefileArgs = (
150150
},
151151
"VERSION" => "2.000010",
152152
"test" => {
153-
"TESTS" => "t/*.t t/JUnit/*.t t/UI/*.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"
153+
"TESTS" => "t/*.t t/JUnit/*.t t/UI/*.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"
154154
}
155155
);
156156

lib/Test2/Harness/Scheduler.pm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,9 @@ sub abort {
307307
CORE::kill('TERM', $pid);
308308
$job->{killed} = 1;
309309
}
310+
311+
# Also terminate the scheduler itself, as BAIL_OUT means we should exit
312+
$self->terminate(1);
310313
}
311314

312315
sub kill {

t/integration/bailout.t

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
use Test2::V0;
2+
# HARNESS-DURATION-LONG
3+
4+
use App::Yath::Tester qw/yath/;
5+
use Test2::Plugin::Immiscible(sub { $ENV{TEST2_HARNESS_ACTIVE} ? 1 : 0 });
6+
7+
my $dir = __FILE__;
8+
$dir =~ s{\.t$}{}g;
9+
$dir =~ s{^\./}{};
10+
11+
# Test that BAIL_OUT causes yath to exit (not hang)
12+
# Regression test for https://github.com/Test-More/Test2-Harness/issues/287
13+
yath(
14+
command => 'test',
15+
args => [$dir, '--ext=tx'],
16+
exit => T(),
17+
test => sub {
18+
my $out = shift;
19+
ok($out->{exit}, "yath exits with non-zero when BAIL_OUT is encountered");
20+
like($out->{output}, qr/BAIL_OUT|bail|halt/i, "output mentions bail/halt");
21+
},
22+
);
23+
24+
# Test that BAIL_OUT is ignored when --no-abort-on-bail is set
25+
yath(
26+
command => 'test',
27+
args => [$dir, '--ext=tx', '--no-abort-on-bail'],
28+
exit => T(),
29+
test => sub {
30+
my $out = shift;
31+
# The test still fails (BAIL_OUT is a failure), but yath should
32+
# not abort the entire suite — just the bailing test fails
33+
ok($out->{exit}, "yath exits non-zero (test still fails)");
34+
},
35+
);
36+
37+
# Test that when BAIL_OUT is disabled via env, everything passes
38+
yath(
39+
command => 'test',
40+
args => [$dir, '--ext=tx'],
41+
env => {BAILOUT_DO_PASS => 1},
42+
exit => 0,
43+
test => sub {
44+
my $out = shift;
45+
ok(!$out->{exit}, "yath exits cleanly when BAIL_OUT is not triggered");
46+
},
47+
);
48+
49+
done_testing;

t/integration/bailout/bail.tx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
use Test2::Tools::Tiny;
2+
use strict;
3+
use warnings;
4+
5+
ok(1, "a passing test before bail");
6+
7+
BAIL_OUT("Something went horribly wrong") unless $ENV{BAILOUT_DO_PASS};
8+
9+
ok(1, "should not reach here");
10+
11+
done_testing;

t/integration/bailout/pass.tx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
use Test2::Tools::Tiny;
2+
use strict;
3+
use warnings;
4+
5+
ok(1, "this test always passes");
6+
7+
done_testing;

t/unit/App/Yath/Command.t

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,84 @@
11
use Test2::V0 -target => 'App::Yath::Command';
22

3-
skip_all "write me";
3+
subtest 'name() derives from package name' => sub {
4+
is(CLASS->name, 'App::Yath::Command', 'base class returns full package name when no prefix match');
5+
6+
{
7+
package App::Yath::Command::testcmd;
8+
use parent 'App::Yath::Command';
9+
}
10+
is(App::Yath::Command::testcmd->name, 'testcmd', 'single-level subclass returns short name');
11+
12+
{
13+
package App::Yath::Command::foo::bar;
14+
use parent 'App::Yath::Command';
15+
}
16+
is(App::Yath::Command::foo::bar->name, 'foo-bar', 'nested namespace uses hyphen separator');
17+
18+
my $obj = CLASS->new();
19+
is($obj->name, 'App::Yath::Command', 'name() works on instance');
20+
};
21+
22+
subtest 'group()' => sub {
23+
is(CLASS->group, 'Z-FIXME', 'base class returns Z-FIXME when no prefix');
24+
25+
{
26+
package App::Yath::Command::sub::cmd;
27+
use parent 'App::Yath::Command';
28+
}
29+
is(App::Yath::Command::sub::cmd->group, undef, 'subcommand (contains hyphen in name) returns undef');
30+
};
31+
32+
subtest 'default metadata' => sub {
33+
is(CLASS->summary, 'No Summary', 'default summary');
34+
is(CLASS->description, 'No Description', 'default description');
35+
};
36+
37+
subtest 'boolean flags default to 0' => sub {
38+
is(CLASS->accepts_dot_args, 0, 'accepts_dot_args defaults to 0');
39+
is(CLASS->args_include_tests, 0, 'args_include_tests defaults to 0');
40+
is(CLASS->internal_only, 0, 'internal_only defaults to 0');
41+
is(CLASS->load_plugins, 0, 'load_plugins defaults to 0');
42+
is(CLASS->load_resources, 0, 'load_resources defaults to 0');
43+
is(CLASS->load_renderers, 0, 'load_renderers defaults to 0');
44+
};
45+
46+
subtest 'cli_args and cli_dot return undef by default' => sub {
47+
is(CLASS->cli_args, undef, 'cli_args returns undef');
48+
is(CLASS->cli_dot, undef, 'cli_dot returns undef');
49+
};
50+
51+
subtest 'run() warns and returns 1' => sub {
52+
my $obj = CLASS->new();
53+
my $warned = 0;
54+
local $SIG{__WARN__} = sub { $warned++ };
55+
my $ret = $obj->run();
56+
is($ret, 1, 'run() returns 1');
57+
is($warned, 1, 'run() emits a warning');
58+
};
59+
60+
subtest 'set_dot_args() croaks' => sub {
61+
my $obj = CLASS->new();
62+
like(
63+
dies { $obj->set_dot_args() },
64+
qr/set_dot_args is not implemented/,
65+
'set_dot_args croaks with expected message',
66+
);
67+
};
68+
69+
subtest 'constructor attributes' => sub {
70+
my $obj = CLASS->new(
71+
settings => { foo => 1 },
72+
args => [qw/a b/],
73+
env_vars => { PATH => '/usr/bin' },
74+
option_state => {},
75+
plugins => [],
76+
);
77+
is($obj->settings, { foo => 1 }, 'settings set via constructor');
78+
is($obj->args, [qw/a b/], 'args set via constructor');
79+
is($obj->env_vars, { PATH => '/usr/bin' }, 'env_vars set via constructor');
80+
is($obj->option_state, {}, 'option_state set via constructor');
81+
is($obj->plugins, [], 'plugins set via constructor');
82+
};
483

584
done_testing;

t/unit/App/Yath/Command/failed.t

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,135 @@
11
use Test2::V0 -target => 'App::Yath::Command::failed';
22

3-
skip_all "write me";
3+
use File::Temp qw/tempdir/;
4+
use Test2::Harness::Util::File::JSONL;
5+
use Getopt::Yath::Settings;
6+
7+
my $dir = tempdir(CLEANUP => 1);
8+
9+
sub make_log {
10+
my (@events) = @_;
11+
my $file = "$dir/test_$$.jsonl";
12+
my $log = Test2::Harness::Util::File::JSONL->new(name => $file);
13+
$log->write(@events);
14+
return $file;
15+
}
16+
17+
sub make_settings {
18+
my (%opts) = @_;
19+
return Getopt::Yath::Settings->new({
20+
failed => { brief => $opts{brief} // 0 },
21+
});
22+
}
23+
24+
subtest 'run() with no failures' => sub {
25+
my $logfile = make_log(
26+
{
27+
stamp => 1,
28+
job_id => 'job1',
29+
facet_data => {
30+
harness_job_end => { rel_file => 't/pass.t', fail => 0 },
31+
},
32+
},
33+
);
34+
35+
my $obj = CLASS->new(
36+
settings => make_settings(),
37+
args => [$logfile],
38+
);
39+
40+
my $ret;
41+
my $out = intercept { $ret = $obj->run() };
42+
is($ret, 0, 'run() returns 0');
43+
};
44+
45+
subtest 'run() detects failed tests' => sub {
46+
my $logfile = make_log(
47+
{
48+
stamp => 1,
49+
job_id => 'job1',
50+
facet_data => {
51+
harness_job_end => { rel_file => 't/fail.t', fail => 1 },
52+
},
53+
},
54+
{
55+
stamp => 2,
56+
job_id => 'job2',
57+
facet_data => {
58+
harness_job_end => { rel_file => 't/pass.t', fail => 0 },
59+
},
60+
},
61+
);
62+
63+
my $obj = CLASS->new(
64+
settings => make_settings(),
65+
args => [$logfile],
66+
);
67+
68+
my $ret;
69+
my $stdout = '';
70+
{
71+
local *STDOUT;
72+
open(STDOUT, '>', \$stdout) or die $!;
73+
$ret = $obj->run();
74+
}
75+
76+
is($ret, 0, 'run() returns 0');
77+
like($stdout, qr/fail\.t/, 'output includes the failed test file');
78+
unlike($stdout, qr/pass\.t/, 'output does not include the passing test file');
79+
};
80+
81+
subtest 'run() brief mode prints only failed filenames' => sub {
82+
my $logfile = make_log(
83+
{
84+
stamp => 1,
85+
job_id => 'job1',
86+
facet_data => {
87+
harness_job_end => { rel_file => 't/fail.t', fail => 1 },
88+
},
89+
},
90+
);
91+
92+
my $obj = CLASS->new(
93+
settings => make_settings(brief => 1),
94+
args => [$logfile],
95+
);
96+
97+
my $ret;
98+
my $stdout = '';
99+
{
100+
local *STDOUT;
101+
open(STDOUT, '>', \$stdout) or die $!;
102+
$ret = $obj->run();
103+
}
104+
105+
is($ret, 0, 'run() returns 0');
106+
like($stdout, qr{t/fail\.t}, 'brief mode prints the failed filename');
107+
};
108+
109+
subtest 'run() dies without a log file argument' => sub {
110+
my $obj = CLASS->new(
111+
settings => make_settings(),
112+
args => [],
113+
);
114+
115+
like(
116+
dies { $obj->run() },
117+
qr/must specify a log file/i,
118+
'dies with helpful message when no log file given',
119+
);
120+
};
121+
122+
subtest 'run() dies for invalid log file' => sub {
123+
my $obj = CLASS->new(
124+
settings => make_settings(),
125+
args => ['/nonexistent/file.jsonl'],
126+
);
127+
128+
like(
129+
dies { $obj->run() },
130+
qr/not a valid log file/,
131+
'dies for nonexistent log file',
132+
);
133+
};
4134

5135
done_testing;

t/unit/App/Yath/Command/help.t

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,46 @@
11
use Test2::V0 -target => 'App::Yath::Command::help';
22

3-
skip_all "write me";
3+
use Getopt::Yath::Settings;
4+
5+
subtest 'run() returns 0 and produces command listing' => sub {
6+
my $settings = Getopt::Yath::Settings->new({
7+
help => { verbose => 0 },
8+
yath => { script => 'yath' },
9+
});
10+
11+
my $obj = CLASS->new(
12+
settings => $settings,
13+
args => [],
14+
);
15+
16+
my $ret;
17+
my $stdout = '';
18+
{
19+
local *STDOUT;
20+
open(STDOUT, '>', \$stdout) or die $!;
21+
$ret = $obj->run();
22+
}
23+
24+
is($ret, 0, 'run() returns 0');
25+
like($stdout, qr/Usage:/, 'output includes Usage line');
26+
like($stdout, qr/COMMANDS:/i, 'output includes COMMANDS section');
27+
like($stdout, qr/help/, 'output lists the help command itself');
28+
};
29+
30+
subtest 'command_table returns formatted string' => sub {
31+
my $settings = Getopt::Yath::Settings->new({
32+
help => { verbose => 0 },
33+
yath => { script => 'yath' },
34+
});
35+
36+
my $obj = CLASS->new(
37+
settings => $settings,
38+
args => [],
39+
);
40+
41+
my $table = $obj->command_table;
42+
ok(defined $table, 'command_table returns a value');
43+
like($table, qr/COMMANDS:/i, 'table contains COMMANDS heading');
44+
};
445

546
done_testing;

0 commit comments

Comments
 (0)