[Bug 28565] New: Adding a new syspref for sharing through HEA should be simpler
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Bug ID: 28565 Summary: Adding a new syspref for sharing through HEA should be simpler Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Right now, we need to add the syspref both in C4::UsageStats and in the UsageStats.t file. The tests should use the list from the lib. To test: 1. Add some syspref to the C4::UsageStats::BuildReport loop 2. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => FAIL: The new syspref is not in the tests, things break -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28563 Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121938 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121938&action=edit Bug 28565: Move the sysprefs to a reusable sub This patch moves the big sysprefs list to a sub. Behavior shouldn't change. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Things work as expected, no changes. 2. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121939 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121939&action=edit Bug 28565: Simplify tests This patch makes the tests for UsageStats. This way the tests don't need to be adjusted everytime a syspref is added. To test: 1. Apply the first patch 2. Add some random syspref to C4::UsageStats 3. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => FAIL: Things explode 4. Apply this patch 5. Repeat 3 => SUCCESS: It doesn't explode anymore! 6. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |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=28565 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=28565 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121938|0 |1 is obsolete| | --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 121986 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121986&action=edit Bug 28565: Move the sysprefs to a reusable sub This patch moves the big sysprefs list to a sub. Behavior shouldn't change. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Things work as expected, no changes. 2. Sign off :-D 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=28565 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121939|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 121987 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121987&action=edit Bug 28565: Simplify tests This patch makes the tests for UsageStats. This way the tests don't need to be adjusted everytime a syspref is added. To test: 1. Apply the first patch 2. Add some random syspref to C4::UsageStats 3. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => FAIL: Things explode 4. Apply this patch 5. Repeat 3 => SUCCESS: It doesn't explode anymore! 6. Sign off :-D 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=28565 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement simplifies release notes| |the way new system | |preferences are added to | |Hea for statistical | |reporting. Before this | |enhancement the tests | |(t/db_dependent/UsageStats. | |t) required adjusting every | |time a new system | |preference was added. Now | |when a new system | |preference is added to Hea | |they are automatically | |picked up for the tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What's the point of this test? :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #5)
What's the point of this test? :D
I was wondering :-D I guess we want the test if we are not leaking things (bug 23849) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- But it's not the test that will catch it, even less if the list is in a single place now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #7)
But it's not the test that will catch it, even less if the list is in a single place now.
I'm fine with dropping the test instead of my patches here. Let me know if you want me to do that. My first instinct was to keep what we have, but make it simpler to maintain. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I would keep the first patch but remove the tests in a second patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121987|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 122289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122289&action=edit Bug 28565: Refactor tests Tests were too complex with no gain. They even deleted the whole DB to run (inside a transaction). I refactored to test the same thing, and make it rely (mostly) on existing data. _count behavior is tested implicitly by comparing with Koha::*->count. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Tests pass 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121986|0 |1 is obsolete| | --- Comment #11 from David Nind <david@davidnind.com> --- Created attachment 122353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122353&action=edit Bug 28565: Move the sysprefs to a reusable sub This patch moves the big sysprefs list to a sub. Behavior shouldn't change. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Things work as expected, no changes. 2. Sign off :-D 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=28565 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122289|0 |1 is obsolete| | --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 122354 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122354&action=edit Bug 28565: Refactor tests Tests were too complex with no gain. They even deleted the whole DB to run (inside a transaction). I refactored to test the same thing, and make it rely (mostly) on existing data. _count behavior is tested implicitly by comparing with Koha::*->count. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Tests pass 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=28565 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=28565 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122353|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 123167 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123167&action=edit Bug 28565: Move the sysprefs to a reusable sub This patch moves the big sysprefs list to a sub. Behavior shouldn't change. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Things work as expected, no changes. 2. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122354|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 123168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123168&action=edit Bug 28565: Refactor tests Tests were too complex with no gain. They even deleted the whole DB to run (inside a transaction). I refactored to test the same thing, and make it rely (mostly) on existing data. _count behavior is tested implicitly by comparing with Koha::*->count. To test: 1. Run: $ kshell k$ prove t/db_dependent/UsageStats.t => SUCCESS: Tests pass 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28565 --- Comment #15 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=28565 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED CC| |kyle@bywatersolutions.com Resolution|--- |FIXED --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Enhancement not backported for 21.05 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org