[Koha-bugs] [Bug 29403] dt_from_string should fail if passed more data than expected for the format

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29403

--- Comment #64 from Jonathan Druart <jonathan.druart+koha at gmail.com> ---
(In reply to Marcel de Rooy from comment #62)
> (In reply to Jonathan Druart from comment #58)
> > (In reply to Marcel de Rooy from comment #48)
> > > -        t::lib::Dates::compare( $updated_on_got, $updated_on_expected,
> > > 'updated_on values matched' );
> > > +        t::lib::Dates::compare(
> > > +            dt_from_string( $updated_on_got,      'rfc3339' ),
> > > +            dt_from_string( $updated_on_expected, 'rfc3339' ),
> > > +            'updated_on values matched'
> > > +        );
> > > 
> > > But what does t::lib::Dates?
> > > 
> > > sub compare {
> > >     my ( $got, $expected ) = @_;
> > >     my $dt_got      = dt_from_string($got);
> > >     my $dt_expected = dt_from_string($expected);
> > >     my $diff = $dt_got->epoch - $dt_expected->epoch;
> > >     if ( abs($diff) <= 5 ) { return 0 }
> > >     return $diff > 0 ? 1 : -1;
> > > }
> > > 
> > > So $got en $expected should be strings, right?
> > > Does this actually show that we miss a parameter in sub compare, and we are
> > > solving it by twisting the test instead?
> > 
> > Will fix on bug 29884.
> 
> That seems to be a good fix, but does not fully address my comment. This
> compare sub runs dt_from_string on its input. So normally I would expect it
> to have string parameters. The patrons.t test here is doing a dt conversion
> already with rfc3339. (Compare calls it again.) It would be more consistent
> if we would pass strings to compare and possibly a type like RFC3339,
> ISO8601, etc.
> I am seeing a hardcoded <=5, could be a parameter too or at least a constant?
> So yes, we are leaving scope here. But compare needs attention, and this
> patch set triggers it.

It's not very nice I agree, but dt_from_string accepts DTs :)

-- 
You are receiving this mail because:
You are watching all bug changes.