Bug 36593 - Add support for the `time` column type on TestBuilder
Summary: Add support for the `time` column type on TestBuilder
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 6796
Blocks:
  Show dependency treegraph
 
Reported: 2024-04-12 11:51 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-07-04 18:36 UTC (History)
6 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
Circulation function:


Attachments
Bug 36593: Add 'time' column type support to TestBuilder (1.75 KB, patch)
2024-04-15 10:09 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 36593: Add missing koha_object(s)_class methods to LibraryHour.pm (bug 9796) (1.01 KB, patch)
2024-04-15 10:09 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 36593: Add 'time' column type support to TestBuilder (1.81 KB, patch)
2024-04-15 13:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36593: Add missing koha_object(s)_class methods to LibraryHour.pm (bug 9796) (1.07 KB, patch)
2024-04-15 13:07 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 Tomás Cohen Arazi (tcohen) 2024-04-12 11:51:44 UTC
Bug 6796 introduced the first use of this DB column type, and TestBuilder is just not ready for it:

https://jenkins.koha-community.org/job/Koha_Master_D10/lastCompletedBuild/console

we should add support.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-04-15 10:09:27 UTC
Created attachment 164871 [details] [review]
Bug 36593: Add 'time' column type support to TestBuilder

This simple patch adds support for the 'time' column type.

To test:
1. Run:
   $ ktd --shell
  k$ perl -Mt::lib::TestBuilder -e 'my $b = t::lib::TestBuilder->new; my
$hour = $b->build_object({ class => "Koha::Library::Hours" });'
=> FAIL: It explodes as it doesn't know how to deal with the 'time'
column type
2. Apply this patch and the follow-up
3. Run:
  k$ prove -MDDP -Mt::lib::TestBuilder -e 'my $b =
t::lib::TestBuilder->new; my $h = $b->build_object({ class => "Koha::Library::Hours" }); p($h->unblessed);'
=> SUCCESS: It generates an hour!
4. Run:
  k$ prove t/db_dependent/TestBuilder.t
=> SUCCESS: It builds all the things!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2024-04-15 10:09:31 UTC
Created attachment 164872 [details] [review]
Bug 36593: Add missing koha_object(s)_class methods to LibraryHour.pm (bug 9796)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Martin Renvoize (ashimema) 2024-04-15 13:07:44 UTC
Created attachment 164881 [details] [review]
Bug 36593: Add 'time' column type support to TestBuilder

This simple patch adds support for the 'time' column type.

To test:
1. Run:
   $ ktd --shell
  k$ perl -Mt::lib::TestBuilder -e 'my $b = t::lib::TestBuilder->new; my
$hour = $b->build_object({ class => "Koha::Library::Hours" });'
=> FAIL: It explodes as it doesn't know how to deal with the 'time'
column type
2. Apply this patch and the follow-up
3. Run:
  k$ prove -MDDP -Mt::lib::TestBuilder -e 'my $b =
t::lib::TestBuilder->new; my $h = $b->build_object({ class => "Koha::Library::Hours" }); p($h->unblessed);'
=> SUCCESS: It generates an hour!
4. Run:
  k$ prove t/db_dependent/TestBuilder.t
=> SUCCESS: It builds all the things!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize (ashimema) 2024-04-15 13:07:47 UTC
Created attachment 164882 [details] [review]
Bug 36593: Add missing koha_object(s)_class methods to LibraryHour.pm (bug 9796)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize (ashimema) 2024-04-15 13:08:31 UTC
Good spot, thanks for the quick follow-up Tomas.

Passing QA directly.
Comment 6 Jonathan Druart 2024-04-15 13:23:04 UTC
I am expecting regressions from this, did you run the whole test suite?

If you set all dates to now, you will have problems in the circulation, holds, etc. logic. Am I missing something?
Comment 7 Tomás Cohen Arazi (tcohen) 2024-04-15 13:25:58 UTC
(In reply to Jonathan Druart from comment #6)
> I am expecting regressions from this, did you run the whole test suite?
> 
> If you set all dates to now, you will have problems in the circulation,
> holds, etc. logic. Am I missing something?

I'm not touching anything for existing behavior. This is replicating it for 'time' only. The only 'time' column is this new one.

Though, the fact TestBuilder cannot build 'Koha::Library::Hour' objects implies... there are no good tests for this feature?
Comment 8 Jonathan Druart 2024-04-15 13:27:20 UTC
Right, ignore me!
Comment 9 Martin Renvoize (ashimema) 2024-04-15 17:23:14 UTC
Well.. yes and no regards tests..

They didn't add the relations so there aren't tests for that and the tests that are added account for changes in Circulation.pm and didn't need mocking..  We could always do with more tests though.. and I'd like to see more of the logic moved back out of controllers into classes.
Comment 10 Katrin Fischer 2024-04-16 16:02:01 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 11 Fridolin Somers 2024-05-23 12:59:38 UTC
Not backported to 23.11.x
Comment 12 Caroline Cyr La Rose 2024-07-04 18:36:20 UTC
Test suite changes don't induce changes to the manual afaik. Closing, but feel free to reopen if the status was put there to document something in the coding guidelines