[Bug 32353] New: items.item_group_id should be undefined in tests by default
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Bug ID: 32353 Summary: items.item_group_id should be undefined in tests by default Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Test Suite Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 24857 TestBuilder should set it to NULL and don't generate the item group. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24857 [Bug 24857] Add ability to group items for records -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |31447 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31447 [Bug 31447] "Please confirm checkout" message uses patron's home library not holds pick up library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144253&action=edit Bug 32353: Don't generate item group for items in tests See bug 31447, we don't want the tests to deal with potential failures because of some item groups. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144254 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144254&action=edit Bug 32353: Pick the default value for FK -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com Assignee|jonathan.druart+koha@gmail. |chris@bigballofwax.co.nz |com | Status|ASSIGNED |NEW --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Entered a rabbit hole here, and found a TestBuilder bug I think. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144254|0 |1 is obsolete| | --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144255 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144255&action=edit Bug 32353: Pick the default value for FK -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Still failing actually... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emmi.takkinen@koha-suomi.fi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + } elsif( !exists $value->{$col_name} + && exists $self->{default_values}{$source}{$col_name} ) { + my $v = $self->{default_values}{$source}{$col_name}; + $v = &$v() if ref($v) eq 'CODE'; + push @$retvalue, $v; } elsif( $col_info->{is_foreign_key} || _should_be_fk($source,$col_name) ) { if( exists $value->{$col_name} ) { if( !defined $value->{$col_name} && !$col_info->{is_nullable} ) { @@ -427,10 +432,6 @@ sub _buildColumnValue { return; } push @$retvalue, $value->{$col_name}; - } elsif( exists $self->{default_values}{$source}{$col_name} ) { - my $v = $self->{default_values}{$source}{$col_name}; - $v = &$v() if ref($v) eq 'CODE'; - push @$retvalue, $v; Sure about this change? This has been working for years? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|jonathan.druart+koha@gmail. |chris@bigballofwax.co.nz |com | --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #6)
Sure about this change? This has been working for years?
We have no default on FK -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #7)
(In reply to Marcel de Rooy from comment #6)
Sure about this change? This has been working for years?
We have no default on FK
A FK might have a default value. We should not make assumptions based on the current state of our tables. If someone adds one, we forget to update something else, etc. etc. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #3)
Entered a rabbit hole here, and found a TestBuilder bug I think.
Creating a TestBuilder bug, I tend to think.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Shouldnt we do something like this: diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm index 5a08b3a018..92068f83c4 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -583,6 +583,8 @@ sub _gen_default_values { damaged => 0, materials => undef, more_subfields_xml => undef, + }, + ItemGroupItem => { item_group_id => undef, }, Category => { There is no such field in items: CREATE TABLE `item_group_items` ( `item_group_items_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'id for the group/item link', `item_group_id` int(11) NOT NULL DEFAULT 0 COMMENT 'foreign key making this table a 1 to 1 join from items to item groups', `item_id` int(11) NOT NULL DEFAULT 0 COMMENT 'foreign key linking this table to the items table', -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- We need a good test here before we change TestBuilder imo. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|items.item_group_id should |reserves.item_group_id |be undefined in tests by |should be undefined in |default |tests by default -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- it's reserves.item_group_id, not items.item_group_id. I've noticed my mistake on Friday, and correct the patch but... hadn't attached it here it seems :-/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144253|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144279 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144279&action=edit Bug 32353: Don't generate item group for holds in tests See bug 31447, we don't want the tests to deal with potential failures because of some item groups. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- # Subtest: transferbook tests 1..9 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 ok 1 - Koha::Patron->store raises an exception on missing params DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 ok 2 - Koha::Patron->store raises an exception on missing params DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 ok 3 - Koha::Patron->store raises an exception on missing params DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 # Looks like you planned 9 tests but ran 3. not ok 57 - transferbook tests # Failed test 'transferbook tests' # at t/db_dependent/Circulation.t line 5394. Exception 'Koha::Exceptions::MissingParameter' thrown 'The to parameter is mandatory' # Looks like your test exited with 11 just after 57. Might be something else ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #14)
Might be something else ?
I think so, it's passing for me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Original problem found with the following warning: Odd number of elements in anonymous hash at t/db_dependent/Circulation.t line 5719. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144255|0 |1 is obsolete| | --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 144957 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144957&action=edit Bug 32353: Pick the default value for FK Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144279|0 |1 is obsolete| | --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 144958 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144958&action=edit Bug 32353: Don't generate item group for holds in tests See bug 31447, we don't want the tests to deal with potential failures because of some item groups. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #19 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Before the patches are applied, ran prove t/db_dependent/Circulation.t 2. Note that tests pass, but there are warnings: root@kohadevbox:koha(bz32353)$ prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 60/64 Odd number of elements in anonymous hash at t/db_dependent/Circulation.t line 5712. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. t/db_dependent/Circulation.t .. ok All tests successful. Files=1, Tests=64, 26 wallclock secs ( 0.15 usr 0.02 sys + 18.55 cusr 2.50 csys = 21.22 CPU) Result: PASS 3. Applied patches and restarted everything. 4. Run test again. Note that "....60/64 Odd number of elements in anonymous hash at t/db_dependent/Circulation.t line 5712." warning/output after running the tests is gone: root@kohadevbox:koha(bz32353)$ prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 60/64 Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. Use of uninitialized value $rentalCharge in numeric gt (>) at /kohadevbox/koha/C4/Circulation.pm line 1119. t/db_dependent/Circulation.t .. ok All tests successful. Files=1, Tests=64, 26 wallclock secs ( 0.13 usr 0.01 sys + 18.61 cusr 2.52 csys = 21.27 CPU) Result: PASS I hope this is sufficient for testing - I don't understand all the other stuff 8-). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144957|0 |1 is obsolete| | Attachment #144958|0 |1 is obsolete| | --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 146516 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146516&action=edit Bug 32353: Pick the default value for FK Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 146517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146517&action=edit Bug 32353: Don't generate item group for holds in tests See bug 31447, we don't want the tests to deal with potential failures because of some item groups. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 147191 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147191&action=edit Bug 32353: Fix erm_agreements.t Agreement.vendor_id has a default value defined in TestBuilder, but here we were assuming there was one. We should then generate it and pass the value when building the agreement objects. The failure was: koha_1 | # Failed test '200 OK' koha_1 | # at t/db_dependent/api/v1/erm_agreements.t line 95. koha_1 | # got: '400' koha_1 | # expected: '200' koha_1 | koha_1 | # Failed test 'exact match for JSON Pointer ""' koha_1 | # at t/db_dependent/api/v1/erm_agreements.t line 95. koha_1 | # Structures begin differing at: koha_1 | # $got = HASH(0x55cde781a480) koha_1 | # $expected = ARRAY(0x55cde75af9c8) koha_1 | # Looks like you failed 2 tests of 32. 400 because we were calling http://localhost:8081/api/v1/erm/agreements?vendor_id=' which returns {"errors":[{"message":"Expected integer - got string.","path":"\/vendor_id"}],"status":400}% -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #23)
Created attachment 147191 [details] [review] Bug 32353: Fix erm_agreements.t
Agreement.vendor_id has a default value defined in TestBuilder, but here we were assuming there was one. We should then generate it and pass the value when building the agreement objects.
The failure was: koha_1 | # Failed test '200 OK' koha_1 | # at t/db_dependent/api/v1/erm_agreements.t line 95. koha_1 | # got: '400' koha_1 | # expected: '200' koha_1 | koha_1 | # Failed test 'exact match for JSON Pointer ""' koha_1 | # at t/db_dependent/api/v1/erm_agreements.t line 95. koha_1 | # Structures begin differing at: koha_1 | # $got = HASH(0x55cde781a480) koha_1 | # $expected = ARRAY(0x55cde75af9c8) koha_1 | # Looks like you failed 2 tests of 32.
400 because we were calling http://localhost:8081/api/v1/erm/agreements?vendor_id=' which returns {"errors":[{"message":"Expected integer - got string.","path":"\/vendor_id"}],"status":400}%
Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|23.05.00 |23.05.00,22.11.04 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32353 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |lucas@bywatersolutions.com Status|Pushed to stable |RESOLVED --- Comment #26 from Lucas Gass <lucas@bywatersolutions.com> --- Missing dependencies for 22.05.x, no backport. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org