Bug 36160 - Use $builder->build_object when creating patrons in Circulation.t
Summary: Use $builder->build_object when creating patrons in Circulation.t
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: David Gustafsson
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-23 18:55 UTC by David Gustafsson
Modified: 2024-05-28 20:29 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06
Circulation function:


Attachments
Bug 36160: Use $builder->build_object for patron objects in Circulation.t (12.51 KB, patch)
2024-02-23 19:01 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 36160: Use $builder->build_object for patron objects in Circulation.t (12.52 KB, patch)
2024-02-26 13:45 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 36160: Use $builder->build_object for patron objects in Circulation.t (12.58 KB, patch)
2024-02-26 14:25 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36160: Use $builder->build_object for patron objects in Circulation.t (12.64 KB, patch)
2024-04-24 13:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Gustafsson 2024-02-23 18:55:43 UTC
Since Patron::update_lastseen was introduced in Bug 15504 circulation tests can protentially crash with: "Invalid value passed, borrowers.updated_on=SCALAR(0x564a85c3e9b0) expected type is datetime" depending of the value of TrackLastPatronActivityTriggers. This probably does not happen on a fresh Koha installation, but did when I ran the tests locally with "Renew an item" activated for TrackLastPatronActivityTriggers.

The reason is that in the Circulation.t tests patron objects are created through Koha::Patron->new instead of using the builder object, and $self->lastseen($now)->store; is called in update_lastseen, on an object that does not have a valid updated_on, resulting in an invalid default value being assigned inKoha/Schema/Loader/mysql.pm.  Possible added future test might also crash if resulting in these patron objects being updated. This patch fixes this issue, the tests still fail for other reasons:

#   Failed test 'transferbook tests'
#   at t/db_dependent/Circulation.t line 5769.
Exception 'Koha::Exceptions::MissingParameter' thrown 'The to parameter is mandatory'

But will create another issue for this if not due to some strange local issues.
Comment 1 David Gustafsson 2024-02-23 19:01:07 UTC
Created attachment 162398 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Renew an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error
Comment 2 David Nind 2024-02-23 19:37:35 UTC
The tests don't fail for me. Is there something else that needs to be done first?
Comment 3 David Gustafsson 2024-02-26 13:45:41 UTC
Created attachment 162434 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Checking out an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error
Comment 4 David Gustafsson 2024-02-26 13:47:37 UTC
(In reply to David Nind from comment #2)
> The tests don't fail for me. Is there something else that needs to be done
> first?

I assumed it was "Renew an item" since "CanBookBeRenewed test" failed, but it seems it's "Checking out an item" that needs to be enabled (I had them both enabled).
Comment 5 Pedro Amorim 2024-02-26 14:25:42 UTC
Created attachment 162435 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Checking out an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2024-04-24 13:57:09 UTC
Created attachment 165461 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Checking out an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-04-24 13:57:40 UTC
More reliable tests is always a win.. thanks David!

Tests continue to make sense and pass, Passing QA
Comment 8 Katrin Fischer 2024-04-26 12:16:57 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 9 Fridolin Somers 2024-05-23 15:24:56 UTC
Pushed to 23.11.x for 23.11.06
Comment 10 Lucas Gass (lukeg) 2024-05-28 20:29:10 UTC
Not needed in 23.05