Summary: | Adding a new syspref for sharing through HEA should be simpler | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi <tomascohen> |
Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | david, jonathan.druart, katrin.fischer, kyle, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28563 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This enhancement simplifies 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.
|
Version(s) released in: |
21.11.00
|
Attachments: |
Bug 28565: Move the sysprefs to a reusable sub
Bug 28565: Simplify tests Bug 28565: Move the sysprefs to a reusable sub Bug 28565: Simplify tests Bug 28565: Refactor tests Bug 28565: Move the sysprefs to a reusable sub Bug 28565: Refactor tests Bug 28565: Move the sysprefs to a reusable sub Bug 28565: Refactor tests |
Description
Tomás Cohen Arazi
2021-06-14 20:43:08 UTC
Created attachment 121938 [details] [review] 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 Created attachment 121939 [details] [review] 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 Created attachment 121986 [details] [review] 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> Created attachment 121987 [details] [review] 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> What's the point of this test? :D (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) But it's not the test that will catch it, even less if the list is in a single place now. (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. I would keep the first patch but remove the tests in a second patch. Created attachment 122289 [details] [review] 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> Created attachment 122353 [details] [review] 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> Created attachment 122354 [details] [review] 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> Created attachment 123167 [details] [review] 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> Created attachment 123168 [details] [review] 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> Pushed to master for 21.11, thanks to everybody involved! Enhancement not backported for 21.05 |