[koha-commits] main Koha release repository branch master updated. v3.22.00-1521-ga8f29dd
Git repo owner
gitmaster at git.koha-community.org
Wed May 4 15:40:51 CEST 2016
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "main Koha release repository".
The branch, master has been updated
via a8f29dd3f218c6519cc29d76f739318471ea9bf6 (commit)
via 9c136f3d767167c887257df6cd659d4e6b752a9a (commit)
via 2862b19918c722a7a0e5cc8151c4ebdd32f13323 (commit)
via 9de3872b907c6771cf8b70ebc303247d8dddb87c (commit)
via df929ef2b4b9c854a22454209553258198e58541 (commit)
via 99672d1f00a8eb4522d4bad16b7e2193d87b6c23 (commit)
from 49acdc73d3efcc3477bfa70bb003d4249e69acfd (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit a8f29dd3f218c6519cc29d76f739318471ea9bf6
Author: Tomas Cohen Arazi <tomascohen at theke.io>
Date: Thu Apr 21 11:23:58 2016 -0300
Bug 16155: (QA followup) fix small bug in t/db_dependent/ILSDI_Services.t
Signed-off-by: Tomas Cohen Arazi <tomascohen at theke.io>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel at gmail.com>
With all patches applied all test pass,
all == git grep -l "use t::lib::TestBuilder" | grep -v -e 'pm$' -e Old | xargs prove
No errors
Signed-off-by: Kyle M Hall <kyle at bywatersolutions.com>
commit 9c136f3d767167c887257df6cd659d4e6b752a9a
Author: Marcel de Rooy <m.de.rooy at rijksmuseum.nl>
Date: Thu Apr 21 09:15:19 2016 +0200
Bug 16155: [QA Follow-up] Add transaction to BiblioFrameworks.t
This unit test does not have a transaction.
It does not need TestBuilder.
Test plan:
[1] Optionally remove records with mfw1, mfw2 from biblio_framework table.
If you ran this test before and it failed, you may have them.
[2] Run the test.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel at gmail.com>
Signed-off-by: Kyle M Hall <kyle at bywatersolutions.com>
commit 2862b19918c722a7a0e5cc8151c4ebdd32f13323
Author: Marcel de Rooy <m.de.rooy at rijksmuseum.nl>
Date: Tue Mar 29 15:32:24 2016 +0200
Bug 16155: Adjust a few other tests
Accounts.t: Only added a line that ensures the MPL branch exists.
AnonymiseIssueHistory.t: Only add a branch to work with.
Barcodes.t: Replaced clear with delete_all.
CalcFine.t: Remove default issuing rule and add one instead of updating.
Holds.t: Add category S in case it would not exist.
Members.t: Replaced clear with delete_all.
MoveItemFromBiblio.t: Replace last _fk construction.
Test plan:
Run these tests. (See note).
Git grep for only_fk, {_fk} and TestBuilder::default_value.
Note: Holds.t does not pass. Tests 9 and 39 fail, but they did already.
not ok 9 - GetReservesFromItemnumber should return a valid borrowernumber
not ok 39 - Test AlterPriority(), move to bottom
So this test needs attention, but on another report please :)
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel at gmail.com>
Signed-off-by: Kyle M Hall <kyle at bywatersolutions.com>
commit 9de3872b907c6771cf8b70ebc303247d8dddb87c
Author: Marcel de Rooy <m.de.rooy at rijksmuseum.nl>
Date: Thu Mar 10 14:42:08 2016 +0100
Bug 16155: Adjust TestBuilder.t
The changes in TestBuilder.pm require some changes in this test.
[1] Tests have been organized under subtests. A few superfluous tests have
been removed. (There is still some overlap between the sections
of overduerules_transport_type and userpermission.)
[2] The results in the build all sources-test are checked one step further.
[3] Tests are added for field length, null values and delete method.
[4] The former defaults from TestBuilder are incorporated in the tests
for userpermission.
Test plan:
Run t/db_dependent/TestBuilder.t
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel at gmail.com>
Signed-off-by: Kyle M Hall <kyle at bywatersolutions.com>
commit df929ef2b4b9c854a22454209553258198e58541
Author: Marcel de Rooy <m.de.rooy at rijksmuseum.nl>
Date: Tue Mar 29 15:02:50 2016 +0200
Bug 16155: Remove a second use from Members_Attributes.t
Test plan:
Run t/db_dependent/Members_Attributes.t
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel at gmail.com>
Signed-off-by: Kyle M Hall <kyle at bywatersolutions.com>
commit 99672d1f00a8eb4522d4bad16b7e2193d87b6c23
Author: Marcel de Rooy <m.de.rooy at rijksmuseum.nl>
Date: Fri Mar 4 15:15:40 2016 +0100
Bug 16155: Composite keys in TestBuilder and more
The number of tests using TestBuilder is gradually growing. Once you
are familiar with its use, you will appreciate it and find yourself
using it when writing new tests. Although it works quite well, some details
still needs some polishing.
While looking at the handling of composite keys in TestBuilder, a number
of related points came up too. This patch finally ended up in setting
the following design goals:
[1] TB should not only look at the first column of a composite FK.
[2] TB should optionally create records for fixed FK values.
[3] TB should create a new record, never change an existing record.
[4] TB should respect auto_increment columns.
[5] Passing a hash for a foreign key should always imply a new record.
[6] Explicitly passing undef for a column should mean NULL.
[7] Add a delete method; it will replace the clear method.
[8] Simplify by removing $default_values, hash key _fk and param only_fk.
The comments below further clarify these points. Note that they refer to
the old behavior (before this patch) unless stated otherwise.
Comments for point 1
====================
Look at:
$builder->build({
source => 'UserPermission',
value => {
borrowernumber => $borrowerno,
module_bit => { flag => 'my flag' },
code => 'will_be_ignored',
},
});
Module_bit and code here are a composite FK to permissions.
TB ignores the value for the code column here and creates a record with a
randomized code.
But if we would pass the hash in the second column of this compound key like:
borrowernumber => $borrowerno,
module_bit => 10,
code => { code => 'new_code' },
TB would now crash when passing the hash for code thru to DBIx.
Comments for point 2
====================
Look at:
$builder->build({
source => 'UserPermission',
value => {
borrowernumber => $borrowerno,
module_bit => 99,
code => 'new_super_tool',
},
});
TB detects a fixed value for the module_bit, continues and will crash on
DBIx if the foreign keys are not found. In this case it would be friendly
to create a missing linked record.
Comments for point 3
====================
Look at:
$builder->build({
source => 'Branch',
value => { branchcode => 'CPL' },
});
If this branch already exists, this call would modify the branch record and
overwrite all columns with randomized values (expected or not). In any case,
it would be safer here to return undef than modifying an existing record.
Comments for point 4
====================
Look at:
$builder->build({
source => 'Borrower',
value => { borrowernumber => '123456789' },
});
If this number would exist, we would update (as earlier). But if this
number does not exist, we would create the record. Although that is
technically possible, I would prefer to have TB respect the auto
increment property of this column.
Comments for point 5
====================
Look at:
$builder->build({
source => 'Item',
value => { homebranch => { branchcode => 'MPL' } },
});
As discussed under point 3, we should actually not pass primary key values
if we expect to build new records. The same also holds for the deeper
(recursive) calls to build when using hashes like here for homebranch.
In this case again an existing record might be overwritten.
Comments for point 6
====================
Look at:
$builder->build({
source => 'Reserve',
value => { itemnumber => undef },
});
As you know, a reserve without an itemnumber is a biblio level hold.
Unfortunately, TB did not care about passing undefs until now. So you would
get an item level hold.
In the new situation TB will respect these explicit undefs, as long as the
corresponding foreign key column is nullable of course.
Comments for point 7
====================
This patch will allow you to delete records created by TB:
my $patron = $builder->build({ source => 'Borrower' });
$builder->delete({ source => 'Borrower', records => $patron });
Or:
$builder->delete({ source => 'Borrower', records => [ $patron, ... ] });
For safety, delete requires you to provide all primary key values in the
passed hashref(s).
Deleting all records in a table via clear is no longer supported and can
still be arranged in one statement.
Comments for point 8
====================
Current use of TestBuilder reveals that $default_values and only_fk
are not really needed. The current $default_values should imo not be in the
module anyway; if you want to use it, you could still pass it to TB:
$builder->build({ ..., value => { %defa, %your_values } });
Only_fk stops at the very last step of saving the top level record while
storing all linked records at the lower levels. Practical use is not
very obvious; it can be easily simulated by one delete statement.
The hash key _fk is now used to store all linked records one or more levels
down. It is used in a few tests to retrieve a value one level down.
Why not retrieve that one value via the database and get rid of the
whole structure?
Implementation
==============
This highlights the main changes:
The $default_value hash (with some hardcoded values for UserPermission)
is removed from the module. It was used by a test in TestBuilder.t and has
been relocated.
The value of $gen_type is returned now by sub _gen_type. (See new.)
The main change in the build method is moving the foreign keys logic to a
new subroutine _create_links. This routine now looks at all columns of a
composite FK. It checks if a linked record exists for passed values, and
it looks at NULL values.
Routine _buildColumnValues is slightly adjusted to allow for passed undef
values. The theoretically endless loop is replaced by three tries. For
composite unique constraints we only check complete sets of values.
Routine _getForeignKeys contains a check to not return the same relation
twice in case of doubled belongs_to relations in the DBIx scheme.
The eval in _storeColumnValues is removed. The autoincrement check in sub
_buildColumnValue got more priority; the handling of foreign keys has been
adjusted and a check for not-nullable columns has been added.
TEST PLAN
=========
This patch only deals with the TestBuilder module itself. In the follow-up
patches TestBuilder.t and a few other unit tests are adjusted.
[1] Do not yet apply this patch. But apply the 'OldBehavior' patch.
Verify that all tests pass. (They cover the first six design goals.)
[2] Apply this patch. Does TestBuilder still compile (perl -c)?
[3] Run the OldBehavior test again. Do all tests fail now? This means
that we got rid of all unwanted side-effects in the list of goals.
[4] Run some other tests that use TestBuilder. (See below.)
Skip the following tests; a follow-up patch deals with them.
t/db_dependent/Accounts.t
t/db_dependent/Barcodes.t
t/db_dependent/Circulation/AnonymiseIssueHistory.t
t/db_dependent/Circulation/CalcFine.t
t/db_dependent/Holds.t
t/db_dependent/Items/MoveItemFromBiblio.t
t/db_dependent/Koha/BiblioFrameworks.t
t/db_dependent/Members.t
t/db_dependent/TestBuilder.t
Signed-off-by: Marcel de Rooy <m.de.rooy at rijksmuseum.nl>
The following tests pass:
t/db_dependent/Acquisition/OrderUsers.t
t/db_dependent/Auth/haspermission.t
t/db_dependent/Barcodes_ValueBuilder.t
t/db_dependent/Budgets.t
t/db_dependent/Category.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/GetTopIssues.t
t/db_dependent/Circulation/IsItemIssued.t
t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
t/db_dependent/Circulation/Returns.t
t/db_dependent/Circulation/TooMany.t
t/db_dependent/Circulation_dateexpiry.t
t/db_dependent/Circulation_transfers.t
t/db_dependent/CourseReserves.t
t/db_dependent/Creators/Lib.t
t/db_dependent/DecreaseLoanHighHolds.t
t/db_dependent/Exporter/Record.t
t/db_dependent/Holds/LocalHoldsPriority.t
t/db_dependent/Holds/RevertWaitingStatus.t:
t/db_dependent/HoldsQueue.t
t/db_dependent/Holidays.t
t/db_dependent/Items.t
t/db_dependent/Items_DelItem.t
t/db_dependent/Koha/Acquisition/Currencies.t
t/db_dependent/Koha/Authorities.t
t/db_dependent/Koha/BiblioFrameworks.t
t/db_dependent/Koha/Cities.t
t/db_dependent/Koha/Libraries.t
t/db_dependent/Koha/Objects.t
t/db_dependent/Koha/Patron/Categories.t
t/db_dependent/Koha/Patron/Images.t
t/db_dependent/Koha/Patron/Messages.t
t/db_dependent/Koha/Patrons.t
t/db_dependent/Koha/SMS_Providers.t
t/db_dependent/Koha_template_plugin_Branches.t
t/db_dependent/Letters.t
t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
t/db_dependent/Members/GetUpcomingMembershipExpires.t
t/db_dependent/Members_Attributes.t
t/db_dependent/Patron/Borrower_Debarments.t
t/db_dependent/Patron/Borrower_Discharge.t
t/db_dependent/Patron/Borrower_Files.t
t/db_dependent/Ratings.t
t/db_dependent/Reports_Guided.t
t/db_dependent/Reserves/GetReserveFee.t
t/db_dependent/Review.t
t/db_dependent/Serials_2.t
t/db_dependent/ShelfBrowser.t
t/db_dependent/Virtualshelves.t
t/db_dependent/api/v1/patrons.t
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel at gmail.com>
Signed-off-by: Kyle M Hall <kyle at bywatersolutions.com>
-----------------------------------------------------------------------
Summary of changes:
t/db_dependent/Accounts.t | 7 +-
t/db_dependent/Barcodes.t | 8 +-
t/db_dependent/Circulation/AnonymiseIssueHistory.t | 11 +-
t/db_dependent/Circulation/CalcFine.t | 7 +
t/db_dependent/Holds.t | 6 +
t/db_dependent/ILSDI_Services.t | 2 +-
t/db_dependent/Items/MoveItemFromBiblio.t | 11 +-
t/db_dependent/Koha/BiblioFrameworks.t | 6 +-
t/db_dependent/Members.t | 3 +-
t/db_dependent/Members/Attributes.t | 2 -
t/db_dependent/TestBuilder.t | 481 +++++++++++---------
t/lib/TestBuilder.pm | 464 +++++++++++--------
12 files changed, 571 insertions(+), 437 deletions(-)
hooks/post-receive
--
main Koha release repository
More information about the koha-commits
mailing list