[Bug 17950] New: Additional housekeeping on Merge.t (after 17913)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Bug ID: 17950 Summary: Additional housekeeping on Merge.t (after 17913) Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: m.de.rooy@rijksmuseum.nl QA Contact: gmcharlt@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |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=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17908 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17908 [Bug 17908] Authority merge omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 59397 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59397&action=edit Bug 17950: Improve quick UNIMARC fix in Merge.t Bug 17909 and 17913 added a quick fix for Merge.t on UNIMARC records. This patch improves that fix with the sub compare_fields, a merge from compare_field_count and compare_field_order. Also it adds the option to test MARC21/UNIMARC by adding a command line switch that triggers mocking the marcflavour preference. The test on a cleared field 609 in strict mode has been broken up in two tests: first a count without 609 and then counting 609s only. Note: Could have mocked GetMarcBiblio too, but decided to go this way. Test plan: [1] Run perl t/db_dependent/Authorities/Merge.t [2] (For UNIMARC users:) Run perl Merge.t -flavour MARC21 [3] (For others:) Run perl Merge.t -flavour UNIMARC 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=17950 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 59398 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59398&action=edit Bug 17950: Merge.t: Improve variable handling at top level This patch does: [1] Remove unused $dbh. [2] Move all mocking statements to one sub. [3] Promote a few lexicals to globals when used in the subtests. Test plan: Run t/db_dependent/Authorities/Merge.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59398|0 |1 is obsolete| | --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 59399 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59399&action=edit Bug 17950: Merge.t: Improve variable handling at top level This patch does: [1] Remove unused $dbh. [2] Move all mocking statements to one sub. [3] Promote a few lexicals to globals when used in the subtests. Test plan: Run t/db_dependent/Authorities/Merge.t 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=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Julian: Would the second patch meet your concerns sufficiently? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Marcel de Rooy from comment #4)
Julian: Would the second patch meet your concerns sufficiently?
Absolutely :) Having the mock objects initialized in the sub where they are used looks better IMO -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Additional housekeeping on |Small improvements for |Merge.t (after 17913) |Merge.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14026 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14026 [Bug 14026] Deleting authority record does not update biblio records -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59397|0 |1 is obsolete| | --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 59689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59689&action=edit Bug 17950: Improve quick UNIMARC fix in Merge.t Bug 17909 and 17913 added a quick fix for Merge.t on UNIMARC records. This patch improves that fix with the sub compare_fields, a merge from compare_field_count and compare_field_order. Also it adds the option to test MARC21/UNIMARC by adding a command line switch that triggers mocking the marcflavour preference. The test on a cleared field 609 in strict mode has been broken up in two tests: first a count without 609 and then counting 609s only. Note: Could have mocked GetMarcBiblio too, but decided to go this way. Test plan: [1] Run perl t/db_dependent/Authorities/Merge.t [2] (For UNIMARC users:) Run perl Merge.t -flavour MARC21 [3] (For others:) Run perl Merge.t -flavour UNIMARC Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> NOTE: prove -v t/db_dependent/Authorities/Merge.t :: -flavor {flavour} Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59399|0 |1 is obsolete| | --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 59690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59690&action=edit Bug 17950: Merge.t: Improve variable handling at top level This patch does: [1] Remove unused $dbh. [2] Move all mocking statements to one sub. [3] Promote a few lexicals to globals when used in the subtests. Test plan: Run t/db_dependent/Authorities/Merge.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com Status|Needs Signoff |Signed Off --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- No need to run perl {testfile} you can use :: to pass parameters to the test file. $ prove -v t/db_dependent/Authorities/Merge.t :: -flavor {flavour} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #9 from Julian Maurice <julian.maurice@biblibre.com> --- I wonder if making a test script parameterizable is the way to go here. Wouldn't it be cool to run the tests for each MARC flavour when you run prove ? :) Not blocking for QA, just a thought. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Julian Maurice from comment #9)
I wonder if making a test script parameterizable is the way to go here. Wouldn't it be cool to run the tests for each MARC flavour when you run prove ? :)
Not blocking for QA, just a thought.
Yes, we could to that. But I prefer to postpone it to another report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Julian Maurice from comment #9)
I wonder if making a test script parameterizable is the way to go here.
It is the most expandable, in terms of we don't know what marcflavors will be valid in the future, and writing code such that you have to change it all the time is kind of annoying. Plus, I agree with Marcel in comment #10. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 --- Comment #12 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to M. Tompsett from comment #11)
It is the most expandable, in terms of we don't know what marcflavors will be valid in the future, and writing code such that you have to change it all the time is kind of annoying.
The list of MARC flavours doesn't change all the time. Plus we have the full list available from systempreferences.options. Anyway, as I said it's not blocking so I'll do QA now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59689|0 |1 is obsolete| | --- Comment #13 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 59917 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59917&action=edit Bug 17950: Improve quick UNIMARC fix in Merge.t Bug 17909 and 17913 added a quick fix for Merge.t on UNIMARC records. This patch improves that fix with the sub compare_fields, a merge from compare_field_count and compare_field_order. Also it adds the option to test MARC21/UNIMARC by adding a command line switch that triggers mocking the marcflavour preference. The test on a cleared field 609 in strict mode has been broken up in two tests: first a count without 609 and then counting 609s only. Note: Could have mocked GetMarcBiblio too, but decided to go this way. Test plan: [1] Run perl t/db_dependent/Authorities/Merge.t [2] (For UNIMARC users:) Run perl Merge.t -flavour MARC21 [3] (For others:) Run perl Merge.t -flavour UNIMARC Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59690|0 |1 is obsolete| | --- Comment #14 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 59918 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59918&action=edit Bug 17950: Merge.t: Improve variable handling at top level This patch does: [1] Remove unused $dbh. [2] Move all mocking statements to one sub. [3] Promote a few lexicals to globals when used in the subtests. Test plan: Run t/db_dependent/Authorities/Merge.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Julian Maurice <julian.maurice@biblibre.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=17950 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17950 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |katrin.fischer@bsz-bw.de --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- These patches have been pushed to 16.11.x and will be in 16.11.04. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org