https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29403 --- Comment #64 from Jonathan Druart <jonathan.druart+koha@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.