[Bug 28479] New: TestBuilder.pm uses incorrect method for checking if objects to be created exists
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 Bug ID: 28479 Summary: TestBuilder.pm uses incorrect method for checking if objects to be created exists Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: minor Priority: P5 - low Component: Test Suite Assignee: joonas.kylmala@helsinki.fi Reporter: joonas.kylmala@helsinki.fi QA Contact: testopia@bugs.koha-community.org CC: jonathan.druart+koha@gmail.com Blocks: 22690 While working on bug 22690 it was discovered that TestBuilder.pm uses incorrect method to check if objects to be built already exists or not. The warning we got due to this was:
DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235
I'm working on a patch for this, I think it helps to explain more the problem – it was quite tough to debug but I think I got to the bottom of it finally. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 [Bug 22690] Merging records with many items too slow (Elasticsearch) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 Joonas Kylmälä <joonas.kylmala@helsinki.fi> 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=28479 --- Comment #1 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 121473 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121473&action=edit Bug 28479: Use primary keys to check object existence in TestBuilder The TestBuilder::build_object function used any foreign keys to check whether an object already exists or not. This brought incorrectly results of unrelated objects because using any other keys other than primary keys don't guarantee our results to point to one single object. For example, as is put here in the unit test, if you created two items with the same biblionumber and then tried to create a hold using build_object() we were using the biblionumber to check whether an item was linked to the hold already. Thus, we were checking whether a random item was already linked to the hold instead of the one we wanted either by passing it explicitly to build_object() or the one build_object() created implicitly. This also resulted in following warnings when there were more than one match: DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 To test: $ prove t/db_dependent -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 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=28479 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121473|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 121501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121501&action=edit Bug 28479: Use primary keys to check object existence in TestBuilder The TestBuilder::build_object function used any foreign keys to check whether an object already exists or not. This brought incorrectly results of unrelated objects because using any other keys other than primary keys don't guarantee our results to point to one single object. For example, as is put here in the unit test, if you created two items with the same biblionumber and then tried to create a hold using build_object() we were using the biblionumber to check whether an item was linked to the hold already. Thus, we were checking whether a random item was already linked to the hold instead of the one we wanted either by passing it explicitly to build_object() or the one build_object() created implicitly. This also resulted in following warnings when there were more than one match: DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 To test: $ prove t/db_dependent 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=28479 Nick Clemens <nick@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=28479 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121501|0 |1 is obsolete| | --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 121858 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121858&action=edit Bug 28479: Use primary keys to check object existence in TestBuilder The TestBuilder::build_object function used any foreign keys to check whether an object already exists or not. This brought incorrectly results of unrelated objects because using any other keys other than primary keys don't guarantee our results to point to one single object. For example, as is put here in the unit test, if you created two items with the same biblionumber and then tried to create a hold using build_object() we were using the biblionumber to check whether an item was linked to the hold already. Thus, we were checking whether a random item was already linked to the hold instead of the one we wanted either by passing it explicitly to build_object() or the one build_object() created implicitly. This also resulted in following warnings when there were more than one match: DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 To test: $ prove t/db_dependent Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.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=28479 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00 |21.11.00,21.05.02 released in| | CC| |kyle@bywatersolutions.com Status|Pushed to master |Pushed to stable --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|21.11.00,21.05.02 |21.11.00,21.05.02,20.11.08 released in| | CC| |fridolin.somers@biblibre.co | |m --- Comment #6 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED CC| |victor@tuxayo.net --- Comment #7 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org