[Bug 14113] New: Silence warnings t/Date.t
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Bug ID: 14113 Summary: Silence warnings t/Date.t Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: mtompset@hotmail.com QA Contact: gmcharlt@gmail.com And clean up the vertical spacing too on the output which is forced on the user. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8879 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor Assignee|chris@bigballofwax.co.nz |mtompset@hotmail.com Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38754 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38754&action=edit Bug 14113: Silence t/Dates.t warnings and cleanup output. The expectation of a user not freaking out over 'Illegal date' messages is not sensible. Catch the messages, give a nicer message. Also, compress the veritical spacing on the output. TEST PLAN --------- 1) $ prove t/Dates.t -- notice Illegal date messages, and extra lines between some sectional messages. 2) Apply patch 3) $ prove t/Dates.t -- Notice how nice and clean it is? Try it with -v to see all the warnings, including the expected warning type messages. 4) run koha qa test tools. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38754|0 |1 is obsolete| | --- Comment #2 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 38904 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38904&action=edit [SIGNED-OFF] Bug 14113: Silence t/Dates.t warnings and cleanup output. The expectation of a user not freaking out over 'Illegal date' messages is not sensible. Catch the messages, give a nicer message. Also, compress the veritical spacing on the output. TEST PLAN --------- 1) $ prove t/Dates.t -- notice Illegal date messages, and extra lines between some sectional messages. 2) Apply patch 3) $ prove t/Dates.t -- Notice how nice and clean it is? Try it with -v to see all the warnings, including the expected warning type messages. 4) run koha qa test tools. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tests run, cleaner output, no koha-qa errors -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |bgkriegel@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 38904 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38904 [SIGNED-OFF] Bug 14113: Silence t/Dates.t warnings and cleanup output. Review of attachment 38904: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14113&attachment=38904) ----------------------------------------------------------------- ::: t/Dates.t @@ +59,3 @@
foreach ( @{ $thash{'iso'} } ) { + if ($_ =~ /1952/) { + warning_like { $val = format_date($_); } qr/Illegal date/,
This is confusing I think. The date is not illegal because it contains 1952, but because the day or month is 0. Maybe should we test all cases (list all the illegal dates), or be explicit somewhere. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #3)
Comment on attachment 38904 [details] [review] [SIGNED-OFF] Bug 14113: Silence t/Dates.t warnings and cleanup output.
Review of attachment 38904 [details] [review]: -----------------------------------------------------------------
::: t/Dates.t @@ +59,3 @@
foreach ( @{ $thash{'iso'} } ) { + if ($_ =~ /1952/) { + warning_like { $val = format_date($_); } qr/Illegal date/,
This is confusing I think. The date is not illegal because it contains 1952, but because the day or month is 0.
Yes, but of the dates in the hash, the only illegal dates have 1952 as the year.
Maybe should we test all cases (list all the illegal dates), or be explicit somewhere.
I didn't want to restructure code. I suppose if we broke the dates into legal and illegal dates and did the same loop twice that would be okay? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to M. Tompsett from comment #4)
(In reply to Jonathan Druart from comment #3)
Comment on attachment 38904 [details] [review] [review] [SIGNED-OFF] Bug 14113: Silence t/Dates.t warnings and cleanup output.
Review of attachment 38904 [details] [review] [review]: -----------------------------------------------------------------
::: t/Dates.t @@ +59,3 @@
foreach ( @{ $thash{'iso'} } ) { + if ($_ =~ /1952/) { + warning_like { $val = format_date($_); } qr/Illegal date/,
This is confusing I think. The date is not illegal because it contains 1952, but because the day or month is 0.
Yes, but of the dates in the hash, the only illegal dates have 1952 as the year.
Yes I understood, but it's confusing :)
Maybe should we test all cases (list all the illegal dates), or be explicit somewhere.
I didn't want to restructure code. I suppose if we broke the dates into legal and illegal dates and did the same loop twice that would be okay?
What about checking /^00?-/, /-0?0$/ and /0{4}$/ ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38904|0 |1 is obsolete| | --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39431 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39431&action=edit Bug 14113: Silence t/Dates.t warnings and cleanup output. The expectation of a user not freaking out over 'Illegal date' messages is not sensible. Catch the messages, give a nicer message. Also, compress the veritical spacing on the output. TEST PLAN --------- 1) $ prove t/Dates.t -- notice Illegal date messages, and extra lines between some sectional messages. 2) Apply patch 3) $ prove t/Dates.t -- Notice how nice and clean it is? Try it with -v to see all the warnings, including the expected warning type messages. 4) run koha qa test tools. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleishaamohia@hotmail.com --- Comment #7 from Aleisha Amohia <aleishaamohia@hotmail.com> --- I still see the Illegal Date messages. Did you want to get rid of these or reword them entirely? If this is the expected outcome then I'm happy to sign off on this. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39431|0 |1 is obsolete| | --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39546 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39546&action=edit Bug 14113: Silence t/Dates.t warnings and cleanup output. The expectation of a user not freaking out over 'Illegal date' messages is not sensible. Catch the messages, give a nicer message. Also, compress the veritical spacing on the output. TEST PLAN --------- 1) $ prove t/Dates.t -- notice Illegal date messages, and extra lines between some sectional messages. 2) Apply patch 3) $ prove t/Dates.t -- Notice how nice and clean it is? Try it with -v to see all the output, including the expected warning type messages. 4) run koha qa test tools. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39546|0 |1 is obsolete| | --- Comment #9 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 39620 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39620&action=edit Bug 14113: Silence t/Dates.t warnings and cleanup output. The expectation of a user not freaking out over 'Illegal date' messages is not sensible. Catch the messages, give a nicer message. Also, compress the veritical spacing on the output. TEST PLAN --------- 1) $ prove t/Dates.t -- notice Illegal date messages, and extra lines between some sectional messages. 2) Apply patch 3) $ prove t/Dates.t -- Notice how nice and clean it is? Try it with -v to see all the output, including the expected warning type messages. 4) run koha qa test tools. Test Remarks : complies with test plan. Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>a -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |indradg@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39620|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 39695 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39695&action=edit [PASSED QA] Bug 14113: Silence t/Dates.t warnings and cleanup output. The expectation of a user not freaking out over 'Illegal date' messages is not sensible. Catch the messages, give a nicer message. Also, compress the veritical spacing on the output. TEST PLAN --------- 1) $ prove t/Dates.t -- notice Illegal date messages, and extra lines between some sectional messages. 2) Apply patch 3) $ prove t/Dates.t -- Notice how nice and clean it is? Try it with -v to see all the output, including the expected warning type messages. 4) run koha qa test tools. Test Remarks : complies with test plan. Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>a Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14113 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Mark! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org