Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB. If they need them, they should create them.
Created attachment 42795 [details] [review] Bug 14878: Tests - Create the branchcodes when needed Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB. If they need them, they should create them. Test plan: Execute the differente test files on a DB without any branchcode or at least without CPL/MPL branches. Confirm that the tests pass. Note for QA: - Not to conflict the list rewrite, this patch don't update VirtualShelves* files. - I am not sure about the changes in t/db_dependent/HoldsQueue.t: +is( $holds_queue->[0]->{cardnumber}, $borrower2->{cardnumber}, "Holds queue filling 1st correct hold for default holds policy 'from home library'"); +is( $holds_queue->[1]->{cardnumber}, $borrower1->{cardnumber}, "Holds queue filling 2nd correct hold for default holds policy 'from home library'"); I have swapped borrower1/borrower2 to make the tests pass, but I didn't understand why...
This patch is quite not esay to test because lot of things remain "db dependent". Jonathan, i did the following (tell me if i'm wrong): - I get the database content provided in bug 10337 (to satisfy all db dependent things), - i created a branchcode 'FOO' on my database and change all borrowers branchcodes with this one, - i deleted all other branches - launch tests on all changed files and get errors on t/db_dependent/Members.t and t/db_dependent/Serials_2.t t/db_dependent/Members.t: Failed test 'Member Changed' (Mismatching member details) t/db_dependent/Serials_2.t: A mysql error
This one is in conflict with bug 15081, that aims to let the test files handle the transactions on their own. I think the transaction should be started/rolled back when the test writer considers it needs to. An example: if we want each subtest to rollback when it finish, DBIx::Class will complain about nested transactions. And if it doesn't rollback explicitly there would be side effects between tests.
Created attachment 44553 [details] [review] Bug 14878: Tests - Create the branchcodes when needed Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB. If they need them, they should create them. Test plan: Execute the differente test files on a DB without any branchcode or at least without CPL/MPL branches. Confirm that the tests pass.
Conflicts with bug 15081 fixed.
Created attachment 44597 [details] [review] Bug 14878: Tests - Create the branchcodes when needed Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB. If they need them, they should create them. Test plan: Execute the differente test files on a DB without any branchcode or at least without CPL/MPL branches. Confirm that the tests pass. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 44598 [details] [review] Bug 14878: (QA followup) Remove useless diag Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Nice job Jonathan!
Created attachment 44601 [details] [review] [PASSED QA] Bug 14878: Tests - Create the branchcodes when needed Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB. If they need them, they should create them. Test plan: Execute the differente test files on a DB without any branchcode or at least without CPL/MPL branches. Confirm that the tests pass. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 44602 [details] [review] [PASSED QA] Bug 14878: (QA followup) Remove useless diag Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patches pushed to master. Thanks Jonathan!