Bug 21134 - Wrong error handling in Koha/Patron/Modification.pm hides a bug
Summary: Wrong error handling in Koha/Patron/Modification.pm hides a bug
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 17767
Blocks:
  Show dependency treegraph
 
Reported: 2018-07-31 15:43 UTC by Jonathan Druart
Modified: 2018-09-21 09:48 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21134: Correctly throw the exception in Koha::Patron::Modification->approve (1.39 KB, patch)
2018-07-31 15:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21134: Correctly throw the exception in Koha::Patron::Modification->approve (1.44 KB, patch)
2018-07-31 16:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-07-31 15:43:24 UTC
koha_1       |     # Looks like you planned 20 tests but ran 9.
koha_1       | 
koha_1       | #   Failed test 'approve tests'
koha_1       | #   at t/db_dependent/Koha/Patron/Modifications.t line 259.
koha_1       | [Something went wrong]# Looks like your test exited with 255 just after 5.
koha_1       | [21:13:47] t/db_dependent/Koha/Patron/Modifications.t ..................... 

"Something went wrong" should never been thrown, Koha::Patron::Modification->approve raises $@, but it's not defined, it should raise $_ instead
Comment 1 Jonathan Druart 2018-07-31 15:44:58 UTC
Created attachment 77342 [details] [review]
Bug 21134: Correctly throw the exception in Koha::Patron::Modification->approve

koha_1       |     # Looks like you planned 20 tests but ran 9.
koha_1       |
koha_1       | #   Failed test 'approve tests'
koha_1       | #   at t/db_dependent/Koha/Patron/Modifications.t line
259.
koha_1       | [Something went wrong]# Looks like your test exited with
255 just after 5.
koha_1       | [21:13:47] t/db_dependent/Koha/Patron/Modifications.t
.....................

"Something went wrong" should never been thrown,
Koha::Patron::Modification->approve raises $@, but it's not defined, it
should raise $_ instead
Comment 2 Jonathan Druart 2018-07-31 15:46:11 UTC
To test you can play with the following snippet:

use Koha::Exceptions::Patron::Modification;
use Try::Tiny;

try {
    die "this is a specific error";
} catch {
    Koha::Exceptions::Patron::Modification->throw($@);
};

then replace $@ with $_
Comment 3 Tomás Cohen Arazi 2018-07-31 16:01:33 UTC
Created attachment 77343 [details] [review]
Bug 21134: Correctly throw the exception in Koha::Patron::Modification->approve

koha_1       |     # Looks like you planned 20 tests but ran 9.
koha_1       |
koha_1       | #   Failed test 'approve tests'
koha_1       | #   at t/db_dependent/Koha/Patron/Modifications.t line
259.
koha_1       | [Something went wrong]# Looks like your test exited with
255 just after 5.
koha_1       | [21:13:47] t/db_dependent/Koha/Patron/Modifications.t
.....................

"Something went wrong" should never been thrown,
Koha::Patron::Modification->approve raises $@, but it's not defined, it
should raise $_ instead

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi 2018-07-31 16:02:09 UTC
Skipping QA for trivial patch.
Comment 5 Martin Renvoize 2018-08-03 13:43:14 UTC
Pushed to 18.05.x for 18.05.03
Comment 6 Fridolin SOMERS 2018-09-21 09:48:54 UTC
Pushed to 17.11.x for 17.11.10