[tapx-dev] [patch] t/080-aggregator.t

Eric Wilhelm scratchcomputing at gmail.com
Wed Aug 1 23:59:56 BST 2007


# from Leif Eriksen
# on Wednesday 01 August 2007 05:21 am:

>I'm _very_ interested in feedback on the quality of this patch

Thanks Leif.  Functionally, it looks great.

A note on maintainability:

Having 'my @die', 'my @warn' and 'my @waits' as file-scoped lexicals 
seems like it might bite someone later.  (e.g. If I'm adding a test and 
say 'my @die' again, I'll only get a warning about 'masks earlier 
declaration'.)

I prefer to scope groups of related tests and their temporary variables 
in their own blocks.

{
  my @die;
  eval {
    local $SIG{__DIE__} = sub {push @die, @_};
    $agg->_get_parsers('no_such_parser_for');
  };

  is @die, 1, ...
}

I think keeping test messages shorter than ~50-60 chars would also be a 
good thing.  When they wrap on the terminal, it gets harder to skim the 
output.

--Eric
-- 
"Time flies like an arrow, but fruit flies like a banana."
--Groucho Marx
---------------------------------------------------
    http://scratchcomputing.com
---------------------------------------------------


More information about the tapx-dev mailing list