Bug 17716

Summary: Remove itemtype-related t/db_dependent/CourseReserves.t warnings
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Test SuiteAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Galen Charlton <gmcharlt>
Severity: normal    
Priority: P5 - low CC: katrin.fischer, kyle, mtompset, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 17716: Remove itemtype-related t/db_dependent/CourseReserves.t
Bug 17716: (followup) Remove dep on existing data and tidy
[SIGNED-OFF] Bug 17716: Remove itemtype-related t/db_dependent/CourseReserves.t
[SIGNED-OFF] Bug 17716: (followup) Remove dep on existing data and tidy
Bug 17716: Remove itemtype-related t/db_dependent/CourseReserves.t
Bug 17716: (followup) Remove dep on existing data and tidy

Description Tomás Cohen Arazi 2016-12-02 18:20:35 UTC

    
Comment 1 Tomás Cohen Arazi 2016-12-02 18:35:34 UTC
Created attachment 57938 [details] [review]
Bug 17716: Remove itemtype-related t/db_dependent/CourseReserves.t

This patch makes t/db_dependent/CourseReserves.t create
good sample data for its tests. It does so by creating a random
itemtype.

To test:
- Run
  $ prove t/db_dependent/CourseReserves.t
=> FAIL: lots of warnings about "item-level_itypes set but no itemtype
  set for item"
- Apply the patch
- Run:
  $ prove t/db_dependent/CourseReserves.t
=> SUCCESS: Tests are green, and no warnings.
- Sign off :-D
Comment 2 Tomás Cohen Arazi 2016-12-02 18:35:39 UTC
Created attachment 57939 [details] [review]
Bug 17716: (followup) Remove dep on existing data and tidy

This patch removes the requirement for this tests for the DB to include
at least 10 borrowers to pass. Borrowers are now created on each run
using t::lib::TestBuilder and a loop.

Bonus: some tiny changes to tidy the file.

To test:
- Run:
  $ prove t/db_dependent/CourseReserves.t
SUCCESS => Tests pass with and without the patch.
- Sign off :-D
Comment 3 Mark Tompsett 2016-12-02 18:57:48 UTC
Comment on attachment 57939 [details] [review]
Bug 17716: (followup) Remove dep on existing data and tidy

Review of attachment 57939 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/CourseReserves.t
@@ -36,5 @@
> -# Create the item
> -my $record = MARC::Record->new();
> -$record->append_fields(
> -    MARC::Field->new( '952', '0', '0', a => $library->{branchcode}, b => $library->{branchcode} )
> -);

Wow. That's strange adding an empty item, but then calling AddItem 2 lines later. Nice catch.
Comment 4 Mark Tompsett 2016-12-02 18:59:43 UTC
Created attachment 57941 [details] [review]
[SIGNED-OFF] Bug 17716: Remove itemtype-related t/db_dependent/CourseReserves.t

This patch makes t/db_dependent/CourseReserves.t create
good sample data for its tests. It does so by creating a random
itemtype.

To test:
- Run
  $ prove t/db_dependent/CourseReserves.t
=> FAIL: lots of warnings about "item-level_itypes set but no itemtype
  set for item"
- Apply the patch
- Run:
  $ prove t/db_dependent/CourseReserves.t
=> SUCCESS: Tests are green, and no warnings.
- Sign off :-D

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 5 Mark Tompsett 2016-12-02 18:59:53 UTC
Created attachment 57942 [details] [review]
[SIGNED-OFF] Bug 17716: (followup) Remove dep on existing data and tidy

This patch removes the requirement for this tests for the DB to include
at least 10 borrowers to pass. Borrowers are now created on each run
using t::lib::TestBuilder and a loop.

Bonus: some tiny changes to tidy the file.

To test:
- Run:
  $ prove t/db_dependent/CourseReserves.t
SUCCESS => Tests pass with and without the patch.
- Sign off :-D

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 Jonathan Druart 2016-12-05 13:24:30 UTC
Created attachment 57960 [details] [review]
Bug 17716: Remove itemtype-related t/db_dependent/CourseReserves.t

This patch makes t/db_dependent/CourseReserves.t create
good sample data for its tests. It does so by creating a random
itemtype.

To test:
- Run
  $ prove t/db_dependent/CourseReserves.t
=> FAIL: lots of warnings about "item-level_itypes set but no itemtype
  set for item"
- Apply the patch
- Run:
  $ prove t/db_dependent/CourseReserves.t
=> SUCCESS: Tests are green, and no warnings.
- Sign off :-D

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 7 Jonathan Druart 2016-12-05 13:24:34 UTC
Created attachment 57961 [details] [review]
Bug 17716: (followup) Remove dep on existing data and tidy

This patch removes the requirement for this tests for the DB to include
at least 10 borrowers to pass. Borrowers are now created on each run
using t::lib::TestBuilder and a loop.

Bonus: some tiny changes to tidy the file.

To test:
- Run:
  $ prove t/db_dependent/CourseReserves.t
SUCCESS => Tests pass with and without the patch.
- Sign off :-D

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 8 Kyle M Hall 2016-12-05 15:33:31 UTC
Pushed to master for 17.05, thanks Tomas!
Comment 9 Katrin Fischer 2016-12-05 20:30:08 UTC
These patches have been pushed to 16.11.x, will be in 16.11.01.