Created attachment 175824 [details] [review] Bug 38761: Add tests prove t/db_dependent/Koha/ILL/Request/Config.t
Created attachment 175825 [details] [review] Bug 38761: Don't pass the 'all' param when getting plugins Test plan: 1) Apply tests patch. Run. Notice it fails 2) Apply this patch. Run tests patch. Notice if passes. UI: 1) Enable ILLModule and install a backend plugin e.g.: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 2) Create an ILL request: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 3) Notice when you click 'New ILL request', 'PluginBackend' is listed even if the plugin is disabled. This patch fixes that. If there are existing requests and then their respective backend is disabled, no error occurs as plugins are loaded in the background even if disabled
Created attachment 175826 [details] [review] Bug 38761: Add tests prove t/db_dependent/Koha/ILL/Request/Config.t
Created attachment 175827 [details] [review] Bug 38761: Don't pass the 'all' param when getting plugins Test plan: 1) Apply tests patch. Run. Notice it fails 2) Apply this patch. Run tests patch. Notice if passes. UI: 1) Enable ILLModule and install a backend plugin e.g.: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 2) Create an ILL request: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 3) Notice when you click 'New ILL request', 'PluginBackend' is listed even if the plugin is disabled. This patch fixes that. If there are existing requests and then their respective backend is disabled, no error occurs as plugins are loaded in the background even if disabled
Tests after both patches applied: - if no ILL backend plugins installed and enabled, the tests fail - if an ILL backend is installed, the tests pass Before the patches were applied (with no ILL backend plugins installed), the tests passed. prove t/db_dependent/Koha/ILL/Request/Config.t t/db_dependent/Koha/ILL/Request/Config.t .. 1/9 # Failed test 'get_backend_plugin_names returns list of backend plugin names' # at t/db_dependent/Koha/ILL/Request/Config.t line 530. # got: undef # expected: 'Test Plugin' # Looks like you failed 1 test of 3. # Failed test 'get_backend_plugin_names() tests' # at t/db_dependent/Koha/ILL/Request/Config.t line 545. # Looks like you failed 1 test of 9. t/db_dependent/Koha/ILL/Request/Config.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/9 subtests Test Summary Report ------------------- t/db_dependent/Koha/ILL/Request/Config.t (Wstat: 256 (exited 1) Tests: 9 Failed: 1) Failed test: 9 Non-zero exit status: 1 Files=1, Tests=9, 1 wallclock secs ( 0.01 usr 0.01 sys + 1.01 cusr 0.17 csys = 1.20 CPU) Result: FAIL
Hi David, thank you. I had the dependencies wrong. Bug 35604 adds the required 'ill_backend' method to Koha/Plugin/Test.pm. Tests should pass now.
Created attachment 175931 [details] [review] Bug 38761: Add tests prove t/db_dependent/Koha/ILL/Request/Config.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 175932 [details] [review] Bug 38761: Don't pass the 'all' param when getting plugins Test plan: 1) Apply tests patch. Run. Notice it fails 2) Apply this patch. Run tests patch. Notice if passes. UI: 1) Enable ILLModule and install a backend plugin e.g.: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 2) Create an ILL request: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 3) Notice when you click 'New ILL request', 'PluginBackend' is listed even if the plugin is disabled. This patch fixes that. If there are existing requests and then their respective backend is disabled, no error occurs as plugins are loaded in the background even if disabled Signed-off-by: David Nind <david@davidnind.com>
Created attachment 176271 [details] [review] Bug 38761: Add tests prove t/db_dependent/Koha/ILL/Request/Config.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 176272 [details] [review] Bug 38761: Don't pass the 'all' param when getting plugins Test plan: 1) Apply tests patch. Run. Notice it fails 2) Apply this patch. Run tests patch. Notice if passes. UI: 1) Enable ILLModule and install a backend plugin e.g.: https://github.com/PTFS-Europe/koha-ill-backend-plugin/releases/tag/v2.0.5 2) Create an ILL request: http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl 3) Notice when you click 'New ILL request', 'PluginBackend' is listed even if the plugin is disabled. This patch fixes that. If there are existing requests and then their respective backend is disabled, no error occurs as plugins are loaded in the background even if disabled Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Test driven development, QA scripts happy, No regressions, bug fixed. Passing QA
This bugfix depends on a bigger feature patch. Any chance we could untangle?
(In reply to Katrin Fischer from comment #12) > This bugfix depends on a bigger feature patch. Any chance we could untangle? I think we can untangle, if we must, but the feature blocking this has sat as SO for 8 months and only recently PQA because we (PTFS-E) have QAd it ourselves. It's its own standalone feature and should not have any impact on current functionality for those upgrading that choose not to use it. Furthermore, we (PTFS-E) have proven time and time again that we are quick to react and fix any eventual issues introduced by our patches (and often times introduced by patches from others). I suppose what I am saying is, we can untangle, but at the same time, can bug 35604 be considered to be reviewed by RM? For push or, ultimately if that's the case, FQA.
I was working through the bug fixes with the stable releases in mind. Of course the other bug will be reviewed, I was only wondering if we could get this one into stable releases independently.