Bug 18420 - Some tests fail without patron category 'S'
Summary: Some tests fail without patron category 'S'
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 18445 18448
  Show dependency treegraph
 
Reported: 2017-04-11 22:38 UTC by Marc Véron
Modified: 2018-06-04 20:10 UTC (History)
4 users (show)

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


Attachments
Bug 18420: Fix Budgets.t when no patron category 'S' exists (1.49 KB, patch)
2017-04-12 14:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists (2.35 KB, patch)
2017-04-12 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18420: Fix Members.t when no patron category 'S' exists (3.16 KB, patch)
2017-04-12 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t (2.06 KB, patch)
2017-04-12 14:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18420: Do not use 'S' as patron category code in other tests (13.66 KB, patch)
2017-04-12 14:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18420: Do not use 'S' as patron category code in other tests (13.53 KB, patch)
2017-04-12 19:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18420: Replace hardcoded EUR in OrderUsers.t (1.24 KB, patch)
2017-04-13 07:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t (2.82 KB, patch)
2017-04-14 10:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Fix Budgets.t when no patron category 'S' exists (1.63 KB, patch)
2017-04-14 19:27 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists (2.48 KB, patch)
2017-04-14 19:28 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Fix Members.t when no patron category 'S' exists (3.30 KB, patch)
2017-04-14 19:31 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t (2.19 KB, patch)
2017-04-14 19:32 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Do not use 'S' as patron category code in other tests (13.66 KB, patch)
2017-04-14 19:34 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Replace hardcoded EUR in OrderUsers.t (1.33 KB, patch)
2017-04-14 19:35 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t (2.91 KB, patch)
2017-04-14 19:37 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18420: Fix Budgets.t when no patron category 'S' exists (1.72 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists (2.58 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Fix Members.t when no patron category 'S' exists (3.39 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t (2.29 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Do not use 'S' as patron category code in other tests (13.75 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Replace hardcoded EUR in OrderUsers.t (1.42 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t (2.95 KB, patch)
2017-04-18 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2017-04-11 22:38:02 UTC
If no patron category 'S' is defined, t/dbdependent/Budgets.t fails.
Comment 1 Marc Véron 2017-04-12 08:14:43 UTC
To reproduce make sure you have no category type 'S' defined but for example 'ST' instead.

Failing:

t/db_dependent/Budgets.t
*) line 583

t/db_dependent/Passwordrecovery.t  
*) line 55

t/db_dependent/Members.t 
*) line 53

*) Test scipts with  line"my $categorycode = 'S';

Passing:

t/db_dependent/Suggestions.t 
lines 70 and following create a category 'S' if it does not exist.
Comment 2 Marcel de Rooy 2017-04-12 11:45:21 UTC
We should use TestBuilder here to create the categories we need.
Comment 3 Jonathan Druart 2017-04-12 14:04:28 UTC
Created attachment 62090 [details] [review]
Bug 18420: Fix Budgets.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Budgets.t
should return green, even if no categories.categorycode 'S' exists
Comment 4 Jonathan Druart 2017-04-12 14:08:09 UTC
Created attachment 62091 [details] [review]
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Passwordrecovery.t
should return green, even if no categories.categorycode 'S' exists
Comment 5 Jonathan Druart 2017-04-12 14:12:22 UTC
Created attachment 62092 [details] [review]
Bug 18420: Fix Members.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Members.t
should return green, even if no categories.categorycode 'S' exists
Comment 6 Jonathan Druart 2017-04-12 14:15:28 UTC
Created attachment 62093 [details] [review]
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t

No need to do it that way, let's use TestBuilder.

Test plan:
  prove t/db_dependent/Suggestions.t
should still return green
Comment 7 Jonathan Druart 2017-04-12 14:16:24 UTC
There are much more left.
Comment 8 Jonathan Druart 2017-04-12 14:50:43 UTC
Created attachment 62095 [details] [review]
Bug 18420: Do not use 'S' as patron category code in other tests

Test plan:
prove all these tests, they must all pass
Comment 9 Marc Véron 2017-04-12 19:37:56 UTC
Problem with:
$ prove t/db_dependent/Acquisition/OrderUsers.t

t/db_dependent/Acquisition/OrderUsers.t .. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`aqorders`, CONSTRAINT `aqorders_currency` FOREIGN KEY (`currency`) REFERENCES `currency` (`currency`) ON DELETE SET NULL ON UPDATE SET NULL) [for Statement "INSERT INTO `aqorders` ( `basketno`, `biblionumber`, `budget_id`, `currency`, `ecost`, `entrydate`, `ordernumber`, `orderstatus`, `quantity`, `quantityreceived`, `rrp`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='7', 1='15', 2='51', 3='EUR', 4=10, 5='01-01-2014', 6=undef, 7=1, 8=2, 9=0, 10=10] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832.
DBIx::Class::Storage::DBI::_dbh_execute(): Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`aqorders`, CONSTRAINT `aqorders_currency` FOREIGN KEY (`currency`) REFERENCES `currency` (`currency`) ON DELETE SET NULL ON UPDATE SET NULL) at /home/marc/koha/Koha/Acquisition/Order.pm line 50
Comment 10 Marc Véron 2017-04-12 19:44:29 UTC
Problem with:
prove t/db_dependent/Holds/LocalHoldsPriority.t

t/db_dependent/Holds/LocalHoldsPriority.t .. 1/6 Odd number of elements in anonymous hash at t/db_dependent/Holds/LocalHoldsPriority.t line 60.
Odd number of elements in anonymous hash at t/db_dependent/Holds/LocalHoldsPriority.t line 60.
Odd number of elements in anonymous hash at t/db_dependent/Holds/LocalHoldsPriority.t line 60.
Odd number of elements in anonymous hash at t/db_dependent/Holds/LocalHoldsPriority.t line 60.
Odd number of elements in anonymous hash at t/db_dependent/Holds/LocalHoldsPriority.t line 60.
DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`reserves`, CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO `reserves` ( `biblionumber`, `branchcode`, `expirationdate`, `found`, `itemnumber`, `itemtype`, `priority`, `reservedate`, `reservenotes`, `waitingdate`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='45', 1='B', 2=undef, 3=undef, 4=undef, 5=undef, 6=1, 7='2017-04-12', 8='', 9=undef] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832.
DBIx::Class::Storage::DBI::_dbh_execute(): Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`reserves`, CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE) at /home/marc/koha/Koha/Object.pm line 121
Comment 11 Jonathan Druart 2017-04-12 19:50:59 UTC
Created attachment 62115 [details] [review]
Bug 18420: Do not use 'S' as patron category code in other tests

Test plan:
prove all these tests, they must all pass
Comment 12 Jonathan Druart 2017-04-12 19:51:37 UTC
t/db_dependent/Acquisition/OrderUsers.t passes for me.
Does it pass for you on master?
Comment 13 Marc Véron 2017-04-12 19:56:08 UTC
(In reply to Jonathan Druart from comment #12)
> t/db_dependent/Acquisition/OrderUsers.t passes for me.
> Does it pass for you on master?

I tested on a fresh install I made when testing the onboarding tool.

Ahh, fails both with and without patches.
Comment 14 Marc Véron 2017-04-12 19:59:13 UTC
(In reply to Marc Véron from comment #10)
> Problem with:
> prove t/db_dependent/Holds/LocalHoldsPriority.t
(...)

Test passes OK with newest patch
Comment 15 Marc Véron 2017-04-12 20:24:17 UTC
(In reply to Marc Véron from comment #13)
> (In reply to Jonathan Druart from comment #12)
> > t/db_dependent/Acquisition/OrderUsers.t passes for me.
> > Does it pass for you on master?
> 
> I tested on a fresh install I made when testing the onboarding tool.
> 
> Ahh, fails both with and without patches.

It seemed that the failing test did not roll back the database, so I restored original VM with install mentioned above.

OK, found:

t/db_dependent/Acquisition/OrderUsers.t line 51 uses EUR as currency but currencies were not yet defined.

Applied patches, created currency EUR, prove t/db_dependent/Acquisition/OrderUsers.t, turns green.
Comment 16 Marcel de Rooy 2017-04-13 06:53:25 UTC
t/db_dependent/Budgets.t (Wstat: 2304 Tests: 142 Failed: 9)
  Failed tests:  30, 44-50, 60
  Non-zero exit status: 9
Comment 17 Marcel de Rooy 2017-04-13 07:04:08 UTC
Created attachment 62118 [details] [review]
Bug 18420: Replace hardcoded EUR in OrderUsers.t
Comment 18 Marcel de Rooy 2017-04-13 07:07:41 UTC
(In reply to Marcel de Rooy from comment #16)
> t/db_dependent/Budgets.t (Wstat: 2304 Tests: 142 Failed: 9)
>   Failed tests:  30, 44-50, 60
>   Non-zero exit status: 9

t/db_dependent/Budgets.t .. 1/142 DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_master`.`vendor_edi_accounts`, CONSTRAINT `vfk_shipment_budget` FOREIGN KEY (`shipment_budget`) REFERENCES `aqbudgets` (`budget_id`)) [for Statement "DELETE FROM aqbudgets"] at t/db_dependent/Budgets.t line 25.

Obviously, this problem also exists without these patches..
Comment 19 Marc Véron 2017-04-13 19:45:37 UTC
Re-testing on current master 16.12.00.024
Database dropped and recreated.
Run through install, mandatory data only. 
No user, library etc. created.
Applied patches
Restart plack
Testing

prove t/db_dependent/Budgets.t OK

prove t/db_dependent/Passwordrecovery.t FAIL (branchcode undefined)

prove t/db_dependent/Members.t OK
prove t/db_dependent/Suggestions.t OK
prove t/db_dependent/Acquisition/OrderUsers.t OK
prove t/db_dependent/Circulation.t OK
prove t/db_dependent/Circulation/IsItemIssued.t OK
prove t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t OK

prove t/db_dependent/Holds/HoldFulfillmentPolicy.t FAIL
1/10 Can't use an undefined value as an ARRAY reference at t/db_dependent/Holds/HoldFulfillmentPolicy.t line 41.

prove t/db_dependent/Holds/HoldItemtypeLimit.t OK
prove t/db_dependent/Holds/LocalHoldsPriority.t OK
prove t/db_dependent/Holds/RevertWaitingStatus.t OK
prove t/db_dependent/Members/Attributes.t OK
prove t/db_dependent/Patron/Borrower_Debarments.t OK
prove t/db_dependent/Patron/Borrower_Files.t OK
prove t/db_dependent/Serials_2.t OK
Comment 20 Marcel de Rooy 2017-04-14 10:49:54 UTC
Created attachment 62164 [details] [review]
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t

We are going out of scope here, but these tests need a branch/item.

Test plan:
Run the adjusted tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2017-04-14 10:52:12 UTC
Marc, could you signoff? I will pass QA after that. Further problems not strictly related to the bug theme, we should move to a new report. A few fixes are strictly out of scope already..
Comment 22 Marc Véron 2017-04-14 19:27:22 UTC
Created attachment 62174 [details] [review]
Bug 18420: Fix Budgets.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Budgets.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 23 Marc Véron 2017-04-14 19:28:12 UTC
Created attachment 62175 [details] [review]
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Passwordrecovery.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 24 Marc Véron 2017-04-14 19:31:26 UTC
Created attachment 62176 [details] [review]
Bug 18420: Fix Members.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Members.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 25 Marc Véron 2017-04-14 19:32:23 UTC
Created attachment 62177 [details] [review]
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t

No need to do it that way, let's use TestBuilder.

Test plan:
  prove t/db_dependent/Suggestions.t
should still return green

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 26 Marc Véron 2017-04-14 19:34:22 UTC
Created attachment 62178 [details] [review]
Bug 18420: Do not use 'S' as patron category code in other tests

Test plan:
prove all these tests, they must all pass

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 27 Marc Véron 2017-04-14 19:35:16 UTC
Created attachment 62179 [details] [review]
Bug 18420: Replace hardcoded EUR in OrderUsers.t

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 28 Marc Véron 2017-04-14 19:37:45 UTC
Created attachment 62180 [details] [review]
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t

We are going out of scope here, but these tests need a branch/item.

Test plan:
Run the adjusted tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 29 Marcel de Rooy 2017-04-18 12:26:01 UTC
Created attachment 62274 [details] [review]
Bug 18420: Fix Budgets.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Budgets.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 30 Marcel de Rooy 2017-04-18 12:26:08 UTC
Created attachment 62275 [details] [review]
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Passwordrecovery.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 31 Marcel de Rooy 2017-04-18 12:26:13 UTC
Created attachment 62276 [details] [review]
Bug 18420: Fix Members.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Members.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 32 Marcel de Rooy 2017-04-18 12:26:17 UTC
Created attachment 62277 [details] [review]
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t

No need to do it that way, let's use TestBuilder.

Test plan:
  prove t/db_dependent/Suggestions.t
should still return green

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 33 Marcel de Rooy 2017-04-18 12:26:21 UTC
Created attachment 62278 [details] [review]
Bug 18420: Do not use 'S' as patron category code in other tests

Test plan:
prove all these tests, they must all pass

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 34 Marcel de Rooy 2017-04-18 12:26:25 UTC
Created attachment 62279 [details] [review]
Bug 18420: Replace hardcoded EUR in OrderUsers.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 35 Marcel de Rooy 2017-04-18 12:26:30 UTC
Created attachment 62280 [details] [review]
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t

We are going out of scope here, but these tests need a branch/item.

Test plan:
Run the adjusted tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 36 Marcel de Rooy 2017-04-18 12:29:31 UTC
I saw a failure on Circulation.t once (Subtest: CanBookBeIssued + AllowMultipleIssuesOnABiblio), but could not reproduce it. But it is not related to this bug anyhow.
Comment 37 Kyle M Hall 2017-04-21 14:55:17 UTC
Pushed to master for 17.05, thanks Jonathan, Marcel!
Comment 38 Katrin Fischer 2017-04-21 22:55:22 UTC
I am having some problems with applying 
acd91afd0c623e75ea9f9d7fb3b57a39bdfb1890
Can you please check and rebase?
Comment 39 Katrin Fischer 2017-05-13 11:32:00 UTC
Still having problems, please rebase!

Applying: Bug 18420: Do not use 'S' as patron category code in other tests
Using index info to reconstruct a base tree...
M	t/db_dependent/Acquisition/OrderUsers.t
M	t/db_dependent/Circulation.t
M	t/db_dependent/Holds/HoldFulfillmentPolicy.t
M	t/db_dependent/Holds/HoldItemtypeLimit.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Holds/HoldItemtypeLimit.t
Auto-merging t/db_dependent/Holds/HoldFulfillmentPolicy.t
Auto-merging t/db_dependent/Circulation.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
Comment 40 Katrin Fischer 2017-07-16 12:08:26 UTC
2 months, no reply. Closing this. Please reopen with rebased patches if you think this should be included in 16.11.x.