[Bug 16607] New: Add verbose flag to builder sub in TestBuilder
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Bug ID: 16607 Summary: Add verbose flag to builder sub in TestBuilder Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement 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=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16500 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add verbose flag to builder |Add quiet flag to builder |sub in TestBuilder |sub in TestBuilder Status|NEW |ASSIGNED Assignee|chris@bigballofwax.co.nz |m.de.rooy@rijksmuseum.nl Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |16500 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16500 [Bug 16500] Catch two warns in TestBuilder.t with warning_like -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 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=16607 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51857 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51857&action=edit Bug 16607: Add quiet flag to builder sub in TestBuilder If you know what you are doing, you sometimes do not like to see the warnings raised by TestBuilder. This patch allows you to add a quiet flag to an individual build call. Test plan: [1] Add a CPL and a MPL branch (if not present). [2] Run t/db_dependent/TestBuilder.t. You should see no difference. The fix from bug 16500 (suppressing the warn already) has been replaced by using the quiet flag. [3] Run t/db_dependent/Accounts.t too. Before this patch, an existing MPL branch also raised a warn. [4] Run t/db_dependent/Items/AutomaticItemModificationByAge.t too. This test included a check for the existence of a CPL branch. The check has been removed. You should see no warning. 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=16607 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51873&action=edit Bug 16607: Follow-up for Holds.t This test adds a patron category and should raise no warning if it already exists. The new quiet parameter is your friend. Test plan: Add patron category S. Run t/db_dependent/Holds.t and see no warnings. 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=16607 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I don't think it's a good idea to hide these warnings. If they appear it means that the developer is doing something wrong. Giving him the ability not to think but just add a quiet flag is not helping him :) For instance, we should not specify the primary (or unique) key, but let TestBuilder generating it, don't you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #3) Thx for looking into this.
I don't think it's a good idea to hide these warnings.
I agree in general. But if you have a closer look, you will see that this is only a one-time, non-recursive hide for one build call. (Generated recursive build calls already are verbose again.)
If they appear it means that the developer is doing something wrong. Giving him the ability not to think but just add a quiet flag is not helping him :) For instance, we should not specify the primary (or unique) key, but let TestBuilder generating it, don't you think?
A developer could of course misuse this flag. He can do much more bad things :) This flag is intended to intelligently suppress one warning. As example: $builder->build( { source => 'Branch', value => { branchcode => 'CPL' }, quiet => 1 }); In this case the developer uses TestBuilder to INSERT IGNORE a CPL branch in just one line. He knows that TestBuilder will not insert it if it is already there and he does not want to see the warning. So he passes the quiet flag. Normally, we do not specify primary keys in TestBuilder, but I would not like to block this specific use. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #4)
In this case the developer uses TestBuilder to INSERT IGNORE a CPL branch in just one line. He knows that TestBuilder will not insert it if it is already there and he does not want to see the warning.
This assumption is wrong, tests should create the data they need to pass and not rely on existing data. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|16500 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16500 [Bug 16500] Catch two warns in TestBuilder.t with warning_like -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add quiet flag to builder |Remove CPL/MPL from two |sub in TestBuilder |unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #5)
(In reply to Marcel de Rooy from comment #4)
In this case the developer uses TestBuilder to INSERT IGNORE a CPL branch in just one line. He knows that TestBuilder will not insert it if it is already there and he does not want to see the warning.
This assumption is wrong, tests should create the data they need to pass and not rely on existing data.
It was a nice flag, do not add it in the future ;) I adjusted the Accounts.t test now. Hilarious actually. Removed CPL/MPL from AutomaticItemModificationByAge.t. The Holds.t patch goes on another report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 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=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51857|0 |1 is obsolete| | Attachment #51873|0 |1 is obsolete| | --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 52302 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52302&action=edit Bug 16607: Remove CPL/MPL from two unit tests Test t/db_dependent/Accounts.t: This test created a MPL branch, but did not use it any more. Test t/db_dependent/Items/AutomaticItemModificationByAge.t: This test still used CPL/MPL branches. Now it uses two new branches. Test plan: [1] Run t/db_dependent/Accounts.t. [2] Run t/db_dependent/Items/AutomaticItemModificationByAge.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=16607 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #52302|0 |1 is obsolete| | --- Comment #8 from Marc Véron <veron@veron.ch> --- Created attachment 52303 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52303&action=edit Bug 16607: Remove CPL/MPL from two unit tests Test t/db_dependent/Accounts.t: This test created a MPL branch, but did not use it any more. Test t/db_dependent/Items/AutomaticItemModificationByAge.t: This test still used CPL/MPL branches. Now it uses two new branches. Test plan: [1] Run t/db_dependent/Accounts.t. [2] Run t/db_dependent/Items/AutomaticItemModificationByAge.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Both tests pass OK Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=16607 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #52303|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52338 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52338&action=edit Bug 16607: Remove CPL/MPL from two unit tests Test t/db_dependent/Accounts.t: This test created a MPL branch, but did not use it any more. Test t/db_dependent/Items/AutomaticItemModificationByAge.t: This test still used CPL/MPL branches. Now it uses two new branches. Test plan: [1] Run t/db_dependent/Accounts.t. [2] Run t/db_dependent/Items/AutomaticItemModificationByAge.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Both tests pass OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for Koha 16.11, thanks Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Status|Pushed to Master |Pushed to Stable --- Comment #11 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.01. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16607 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com Depends on| |16155, 11023 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11023 [Bug 11023] Automatic item modification by age (Was "Toggle new status for items") https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16155 [Bug 16155] Composite keys in TestBuilder and more -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org