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


Attachments
Bug 36593: Add 'time' column type support to TestBuilder (1.75 KB, patch)
2024-04-15 10:09 UTC, Tomás Cohen Arazi
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
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
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
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 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 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 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 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 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 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 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 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!