| Subject: | return value from add with COMMAND doesn't return correct values |
Hello,
When using a COMMAND to schedule a job, the return value for add
doesn't meet the documentation.
The problem is that open(...) or return 1 is the incorrect way to
detect if the pipe to "at" has failed.
I attach a patch that fixes the problem. Note that SIG{'PIPE'} is
ignored, because if the open fails and we continued writing to the
ATCMD, the parent process (the caller) would die. On close, the $?
variable is properly set to the exit code of the "at" command.
The patch also patches the test cases in two aspects:
1- use Test::More, that way I could use cmp_ok to diagnose failures better
2- test the case when at will return non-zero (enqueing a job in the past)
Note: the patch applies to v1.08 of this module.
| Subject: | schedule-at.diff |
Index: Schedule-At-1.08/At.pm
===================================================================
--- Schedule-At-1.08/At.pm (revision 3605)
+++ Schedule-At-1.08/At.pm (revision 3606)
@@ -52,13 +52,17 @@
if ($params{FILE}) {
return (system($command) / 256);
} else {
- open (ATCMD, "| $command") or return 1;
- print ATCMD "$TAGID$params{TAG}\n" if $params{TAG};
+ {
+ local $SIG{'PIPE'} = 'IGNORE';
+ open ATCMD, "| $command";
+ print ATCMD "$TAGID$params{TAG}\n" if ($params{TAG});
- print ATCMD ref($params{COMMAND}) eq "ARRAY" ?
- join("\n", @{$params{COMMAND}}) : $params{COMMAND};
+ print ATCMD (ref($params{COMMAND}) eq "ARRAY" ?
+ join("\n", @{$params{COMMAND}}) : $params{COMMAND});
- close (ATCMD);
+ close ATCMD; #or die "Can't close to at $!";
+ return $?;
+ }
}
0;
Index: Schedule-At-1.08/t/t1.t
===================================================================
--- Schedule-At-1.08/t/t1.t (revision 3605)
+++ Schedule-At-1.08/t/t1.t (revision 3606)
@@ -1,10 +1,13 @@
-use Test;
+
+use Test::More;
+
+use warnings;
use strict;
BEGIN {
if ($< == 0 || $> == 0 || $ENV{'AT_CAN_EXEC'}) {
$main::FULL_TEST = 1;
- plan tests => 11;
+ plan tests => 13;
} else {
plan tests => 1;
}
@@ -32,18 +35,19 @@
my %afterJobs = Schedule::At::getJobs();
listJobs('Added new job') if $verbose;
-ok(!$rv && ((scalar(keys %beforeJobs)+1) == scalar(keys %afterJobs)));
+ok(!$rv && ((scalar(keys %beforeJobs)+1) == scalar(keys %afterJobs)), 'Enqueued a job for next year');
my %atJobs = Schedule::At::getJobs();
-ok(%atJobs);
+ok(scalar(keys %afterJobs) > 0, 'Got a job list with more that one entry');
+
my ($jobid, $content) = Schedule::At::readJobs(TAG => '_TEST_aTAG');
-ok($content, '/thisIsACommand/');
+like($content, qr/thisIsACommand/, 'Got the job with the tag');
$rv = Schedule::At::remove (TAG => '_TEST_aTAG');
my %afterRemoveJobs = Schedule::At::getJobs();
listJobs('Schedule::At jobs deleted') if $verbose;
-ok(scalar(keys %beforeJobs) == scalar(keys %afterRemoveJobs));
+ok(scalar(keys %beforeJobs) == scalar(keys %afterRemoveJobs), 'Removed the job');
# getJobs with TAG param
$rv = Schedule::At::add (
@@ -78,13 +82,13 @@
my %job = Schedule::At::readJobs(TAG => '1234567890' x 30);
-ok(scalar(keys(%job)),1, 'Conserved 300 chars of data');
+cmp_ok(scalar(keys(%job)), '==', 1, 'Conserved 300 chars of data');
-%job = Schedule::At::remove (TAG => '1234567890' x 30);
-ok(scalar(keys(%job)),1, 'Removed via the tag');
+Schedule::At::remove (TAG => '1234567890' x 30);
+cmp_ok(scalar(keys(%job)), '==', 1, 'Removed via the tag');
-my %job = Schedule::At::readJobs(TAG => '1234567890' x 30);
-ok(scalar(keys(%job)),0, 'Was really removed');
+%job = Schedule::At::readJobs(TAG => '1234567890' x 30);
+cmp_ok(scalar(keys(%job)), '==', 0, 'Was really removed');
sub listJobs {
print STDERR "@_\n" if @_;
@@ -94,3 +98,17 @@
($job->{TAG} || ''), "\n";
}
}
+
+
+my $lastYear = (localtime)[5] + 1900 - 1;
+$rv = Schedule::At::add (
+ TIME => $lastYear . '01181530',
+ COMMAND => 'ls /thisIsACommand/',
+ TAG => '_TEST_pastTAG'
+);
+
+my %pastJobs = Schedule::At::readJobs(TAG => '_TEST_pastTAG');;
+cmp_ok($rv, '!=', 0, 'Got return != 0 for scheduling a task in the past');
+#diag("Got RV $rv");
+#listJobs();
+cmp_ok(scalar(keys(%pastJobs)), '==', 0, 'Task never enqueued');