view release on metacpan or  search on metacpan

t/232-claim-timeout-zero-never-expires.t  view on Meta::CPAN

# t/232-claim-timeout-zero-never-expires.t
#
# Ticket #232. `claim_timeout: 0s` is documented as "claims never expire" --
# in App::karr::Role::ClaimTimeout/claim_timeout_secs, in the comment over
# _parse_timeout, and by the implementation it is parity with (kanban-md's
# internal/board/filter.go, whose IsUnclaimed asks `timeout > 0 && ClaimedAt !=
# nil` and falls through to "still claimed" on a zero timeout).
#
# It did the exact opposite. _parse_timeout handed the zero through verbatim,
# which is right, and _claim_expired then asked `(now - claimed_at) > 0` -- true
# of every claim more than a second old. So the one setting a board uses to say
# "claims are binding here" was the setting that made every card free for the
# taking, seconds after it was claimed, with nothing said anywhere:
#
#     karr config set claim_timeout 0s
#     karr move 1 in-progress --claim agent-a
#     karr pick --claim agent-b
#     -> Picked task 1: A (claimed by agent-b)
#
# Four things are pinned here, and the last three matter as much as the first:
#
#   * zero means never: pick, edit and move all leave a claimed card alone, and
#     expired_claim_report has nothing to report because nothing expired;
#   * a real timeout still expires, and taking the claim over is still
#     announced (#177) -- the fix must not be "no claim ever expires again";
#   * a negative or unparseable claim_timeout still falls back to one hour
#     rather than reaching the new guard and reading as "never";
#   * the same zero on the lock side -- App::karr::Lock/expired, reached from
#     `lock_timeout: 0s` -- answers the same way. Those are two separate
#     guards that have to agree, and drifting apart is what produced this
#     ticket: the lock side read its zero correctly for as long as the claim
#     side read it backwards, because they only look alike. Both answers are
#     asserted in this one file so losing either of them goes red here.
#
# Never the developer's own board: every repository below is a tempdir.
use strict;
use warnings;
use Test::More;
use lib 't/lib';
use TestGit qw( require_git_c );
require_git_c();
use TestKarr qw( run_karr );
use MockStore;
use File::Temp qw( tempdir );
use Time::Piece;

use App::karr::Config;
use App::karr::Git;
use App::karr::Lock;
use App::karr::Task;

my $HOLDER   = 'agent-holder';
my $TAKEOVER = 'agent-takeover';

{
  package ZeroTimeoutConsumer;
  use Moo;
  # The three names the role requires. store is the real question here: unlike
  # t/72's stub, check_claim below has to read a board's claim_timeout back out
  # of it, so this one is a MockStore carrying the value under test.
  has store => ( is => 'ro' );
  sub json  { 0 }
  sub quiet { 0 }
  with 'App::karr::Role::ClaimTimeout';
}

sub _consumer_for {
  my ($timeout) = @_;
  my $ec = App::karr::Config->default_config;
  $ec->{claim_timeout} = $timeout;
  return ZeroTimeoutConsumer->new( store => MockStore->new( ec => $ec ) );
}

sub _claimed_secs_ago {
  my ($secs) = @_;
  return App::karr::Task->new(
    id         => 1,
    title      => 'Held card',
    status     => 'in-progress',
    claimed_by => $HOLDER,
    claimed_at => gmtime( time - $secs )->datetime . 'Z',
  );
}

subtest 'the comparison itself: zero is not a very short window' => sub {
  my $c   = _consumer_for('1h');
  my $old = _claimed_secs_ago(7200);

  # The counter-probe first, so "nothing ever expires" cannot pass this file:
  # a two-hour-old claim under the default one-hour window is expired, and
  # every assertion below is the same task judged against a different number.
  ok( $c->_claim_expired( $old, 3600 ),
    'a two-hour-old claim has expired under a 1h timeout' );

  ok( !$c->_claim_expired( $old, 0 ),
    'the same claim has NOT expired under a timeout of 0 -- zero disables expiry' );
  ok( !$c->_claim_expired( _claimed_secs_ago(86400 * 30), 0 ),
    'and neither has one a month old: zero is never, not sooner' );

  # Answered like zero rather than like a duration, exactly as
  # App::karr::Lock/expired answers a negative ttl. No board can produce this
  # (see the _parse_timeout subtest), but "not a positive window" is the whole
  # question the guard asks.
  ok( !$c->_claim_expired( $old, -1 ),
    'a negative timeout disables expiry too' );

  ok( !$c->_claim_expired( _claimed_secs_ago(60), 3600 ),
    'a fresh claim under a real window is still live' );
};

subtest 'the parse: which values ever reach that comparison' => sub {
  my $c = _consumer_for('1h');

  is( $c->_parse_timeout('0s'), 0,
    '0s is honoured verbatim -- it must reach the comparison as a zero' );
  is( $c->claim_timeout_secs, 3600, 'a board saying 1h reads back as 3600' );
  is( _consumer_for('0s')->claim_timeout_secs, 0,
    'a board saying 0s reads back as 0, not as the fallback' );

  # The fallback still catches everything that is not a duration, so the new
  # guard is never reached by an accident. A negative one especially: it parses
  # and would otherwise be read as "claims never expire" on a board that meant
  # nothing of the sort.
  is( $c->_parse_timeout('-5m'), 3600, 'a negative duration falls back to 1h' );
  is( _consumer_for('-5m')->claim_timeout_secs, 3600,
    '...on the board level too, so it never lands on the zero guard' );
  is( $c->_parse_timeout('7d'),    3600, 'an unsupported unit falls back to 1h' );
  is( $c->_parse_timeout('later'), 3600, 'a non-duration falls back to 1h' );
  is( $c->_parse_timeout('0'),     3600,
    'a bare 0 with no unit keeps its historical fallback, unchanged by this fix' );
  is( $c->_parse_timeout(undef),   3600, 'an absent value falls back to 1h' );