| Summary: | Wrong warning "Invalid date ... passed to output_pref" can be carped | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | f.demians, julian.maurice, kyle, srdjan |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Crowdfunding goal: | 0 |
| Patch complexity: | Small patch | Documentation contact: | |
| Documentation submission: | Text to go in the release notes: | ||
| Version(s) released in: | Circulation function: | ||
| Attachments: |
Bug 16848: Prevent invalid warning to be carped from output_pref
[SIGNED-OFF] Bug 16848: Prevent invalid warning to be carped from output_pref [PASSED QA] Bug 16848: Prevent invalid warning to be carped from output_pref |
||
Created attachment 53090 [details] [review] Bug 16848: Prevent invalid warning to be carped from output_pref From Koha::DateUtils::output_pref: $dt = eval { dt_from_string( $str ) } if $str; carp "Invalid date '$str' passed to output_pref\n" if $@; This second line is wrong: if $str does not exist, the first line is not evaluated and $@ could be filled with previous error. To reproduce: Then: prove t/DateUtils.t will display: t/DateUtils.t .. 20/60 Use of uninitialized value $str in concatenation (.) or string at Koha/DateUtils.pm line 217. Invalid date '' passed to output_pref at t/DateUtils.t line 233. t/DateUtils.t .. ok All tests successful. Files=1, Tests=60, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.40 cusr 0.00 csys = 1.42 CPU) Result: PASS Test plan: Without this patch, you should not see the carp Created attachment 53120 [details] [review] [SIGNED-OFF] Bug 16848: Prevent invalid warning to be carped from output_pref From Koha::DateUtils::output_pref: $dt = eval { dt_from_string( $str ) } if $str; carp "Invalid date '$str' passed to output_pref\n" if $@; This second line is wrong: if $str does not exist, the first line is not evaluated and $@ could be filled with previous error. To reproduce: Then: prove t/DateUtils.t will display: t/DateUtils.t .. 20/60 Use of uninitialized value $str in concatenation (.) or string at Koha/DateUtils.pm line 217. Invalid date '' passed to output_pref at t/DateUtils.t line 233. t/DateUtils.t .. ok All tests successful. Files=1, Tests=60, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.40 cusr 0.00 csys = 1.42 CPU) Result: PASS Test plan: Without this patch, you should not see the carp Signed-off-by: Srdjan <srdjan@catalyst.net.nz> I took liberty of adding local $@ (In reply to Srdjan Jankovic from comment #3) > I took liberty of adding local $@ Are you sure it is useful? Probably not that much, but I have a habit of doing that, I believe it is a good practice. Jonathan, Srdjan, is this ready for QA? I think so, unless Jonathan fiercely objects, in which case I'll remove the line that I've added. (In reply to Srdjan Jankovic from comment #7) > I think so, unless Jonathan fiercely objects, in which case I'll remove the > line that I've added. It should not hurt, so I am ok with it. Created attachment 54121 [details] [review] [PASSED QA] Bug 16848: Prevent invalid warning to be carped from output_pref From Koha::DateUtils::output_pref: $dt = eval { dt_from_string( $str ) } if $str; carp "Invalid date '$str' passed to output_pref\n" if $@; This second line is wrong: if $str does not exist, the first line is not evaluated and $@ could be filled with previous error. To reproduce: Then: prove t/DateUtils.t will display: t/DateUtils.t .. 20/60 Use of uninitialized value $str in concatenation (.) or string at Koha/DateUtils.pm line 217. Invalid date '' passed to output_pref at t/DateUtils.t line 233. t/DateUtils.t .. ok All tests successful. Files=1, Tests=60, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.40 cusr 0.00 csys = 1.42 CPU) Result: PASS Test plan: Without this patch, you should not see the carp Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 16.11, thanks Jonathan! Pushed in 16.05. Will be in 16.05.03. Pushed in 16.05. Will be in 16.05.03. Pushed to 3.22.x, will be in 3.22.10 |
From Koha::DateUtils::output_pref: $dt = eval { dt_from_string( $str ) } if $str; carp "Invalid date '$str' passed to output_pref\n" if $@; This second line is wrong: if $str does not exist, the first line is not evaluated and $@ could be filled with previous error. To reproduce: diff --git a/t/DateUtils.t b/t/DateUtils.t index eede4a0..2dd6a9d 100755 --- a/t/DateUtils.t +++ b/t/DateUtils.t @@ -229,3 +229,5 @@ is( $output_for_invalid_date, undef, 'output_pref should return undef if an inva warning_is { output_pref( { 'str' => $testdate_iso, dt => $dt, dateformat => 'iso', dateonly => 1 } ) } { carped => 'output_pref should not be called with both dt and str parameters' }, 'output_pref should carp if str and dt parameters are passed together'; + +output_pref(); Then: prove t/DateUtils.t will display: t/DateUtils.t .. 20/60 Use of uninitialized value $str in concatenation (.) or string at Koha/DateUtils.pm line 217. Invalid date '' passed to output_pref at t/DateUtils.t line 233. t/DateUtils.t .. ok All tests successful. Files=1, Tests=60, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.40 cusr 0.00 csys = 1.42 CPU) Result: PASS