[Bug 17726] New: TestBuilder's refactoring removed default values
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Bug ID: 17726 Summary: TestBuilder's refactoring removed default values Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Reading the commit message of bug 16155, it was expected to remove the default values for given fields. But actually I think we should reintroduce it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=17727 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- See bug 17727 for a use case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- The way is was hardcoded in the past, was not particularly attractive; removal made the code simpler. You can always pass a value via the value parameter. In this specific case of marcxml field, we should probably call a routine that ensures the consistency between the marcxml and related fields (outside the namespace of TestBuilder). That said, I would not mind reintroducing a (better) default.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I am a bit worried about adding this default value to all the tests creating an item (and track the future ones not to forget to pass the default value). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #3)
I am a bit worried about adding this default value to all the tests creating an item (and track the future ones not to forget to pass the default value).
Yeah. Maybe we should not call _gen_text for a "xml field" but create some 'random' xml that fits better.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=17726 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58152 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58152&action=edit Bug 17726: TestBuilder - Add default values The items.more_subfields_xml is set to random data (generated by TestBuilder), and so GetMarcBiblio does not manage to embed items (if needed). The error is: :1: parser error : Start tag expected, '<' not found More precisely it explodes in C4::Items::_parse_unlinked_item_subfields_from_xml when MARC::Record->new_from_xml is called with an invalid xml This patch adds a default values mechanism to TestBuilder to avoid modifying all the existing calls. Test plan: Set SearchEngine to ElasticSearch prove t/db_dependent/Circulation.pl should return green with this patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 17727 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=17726 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58152|0 |1 is obsolete| | --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 58155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58155&action=edit Bug 17726: TestBuilder - Add default values The items.more_subfields_xml is set to random data (generated by TestBuilder), and so GetMarcBiblio does not manage to embed items (if needed). The error is: :1: parser error : Start tag expected, '<' not found More precisely it explodes in C4::Items::_parse_unlinked_item_subfields_from_xml when MARC::Record->new_from_xml is called with an invalid xml This patch adds a default values mechanism to TestBuilder to avoid modifying all the existing calls. Test plan: Set SearchEngine to ElasticSearch prove t/db_dependent/Circulation.pl should return green with this patch Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 58156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58156&action=edit Bug 17726: [QA Follow-up] Add test descriptions Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Joubu: Please adjust in view of marcxml not being in Biblioitem any more -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 59300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59300&action=edit Bug 17726: biblioitems.marc has been moved to biblio_metadata.metadata -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #10)
Created attachment 59300 [details] [review] Bug 17726: biblioitems.marc has been moved to biblio_metadata.metadata
Thanks. But we still have a problem, Houston: # Subtest: Build all sources 1..1 DBD::mysql::st execute failed: Column 'metadata' cannot be null [for Statement "INSERT INTO `biblio_metadata` ( `biblionumber`, `format`, `marcflavour`, `metadata`) VALUES ( ?, ?, ?, ? )" with ParamValues: 0='5051', 1='wjOZm7m', 2='r8zqCCTHTdruku', 3=undef] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. We have a constraint here.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59300|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 59341 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59341&action=edit Bug 17726: biblioitems.marc has been removed We will certainly have to do something with the biblio_metadata.metadata field later. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Not sure that will be enough, but I do not see a quick solution for now. Tests seem to pass now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59341|0 |1 is obsolete| | --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 59353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59353&action=edit Bug 17726: biblioitems.marc has been removed We will certainly have to do something with the biblio_metadata.metadata field later. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Taking the liberty to pass qa for this one now. No problems detected with this change in the t/db_dependent tests. Does not interfere with the interface, only TestBuilder and tests; not used outside 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=17726 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Jonathan, Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am in doubt about the last patch here - description says marc (true for 16.11) but patch says marcxml (untrue for 16.11). Can someone clarify if this should be included in the next 16.11.x release? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #17)
I am in doubt about the last patch here - description says marc (true for 16.11) but patch says marcxml (untrue for 16.11).
Can someone clarify if this should be included in the next 16.11.x release?
The last patch is not for 16.11. If you have a failing Circulation test in 16.11, the first two might help you solve it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We got a happy Jenkins for 16.11.x and locally the tests pass for me as well - so there seems no need atm. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- They certainly fail if SearchEngine == ElasticSearch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- First 2 patches pushed to 16.11.x, they will be in 16.11.04. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17726 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #22 from Mason James <mtj@kohaaloha.com> --- (In reply to Katrin Fischer from comment #21)
First 2 patches pushed to 16.11.x, they will be in 16.11.04.
First 2 patches pushed to 16.05.x, for 16.05.10 release -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org