Summary: | TestBuilder's refactoring removed default values | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | kyle, m.de.rooy, mtj |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17727 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 17726: TestBuilder - Add default values
Bug 17726: TestBuilder - Add default values Bug 17726: [QA Follow-up] Add test descriptions Bug 17726: biblioitems.marc has been moved to biblio_metadata.metadata Bug 17726: biblioitems.marc has been removed Bug 17726: biblioitems.marc has been removed |
Description
Jonathan Druart
2016-12-06 09:50:23 UTC
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.. 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). (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.. Created attachment 58152 [details] [review] 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 *** Bug 17727 has been marked as a duplicate of this bug. *** Created attachment 58155 [details] [review] 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> Created attachment 58156 [details] [review] Bug 17726: [QA Follow-up] Add test descriptions Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Joubu: Please adjust in view of marcxml not being in Biblioitem any more Created attachment 59300 [details] [review] Bug 17726: biblioitems.marc has been moved to biblio_metadata.metadata (In reply to Jonathan Druart from comment #10) > Created attachment 59300 [details] [review] [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.. Created attachment 59341 [details] [review] Bug 17726: biblioitems.marc has been removed We will certainly have to do something with the biblio_metadata.metadata field later. Not sure that will be enough, but I do not see a quick solution for now. Tests seem to pass now. Created attachment 59353 [details] [review] 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> 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. Pushed to master for 17.05, thanks Jonathan, Marcel! 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? (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. We got a happy Jenkins for 16.11.x and locally the tests pass for me as well - so there seems no need atm. They certainly fail if SearchEngine == ElasticSearch. First 2 patches pushed to 16.11.x, they will be in 16.11.04. (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 |