Commit graph

248 commits

Author SHA1 Message Date
6eeb9bc1b3 Bug 28786: Two-factor authentication for staff client - TOTP
This patchset introduces the Two-factor authentication (2FA) idea in
Koha.

It is far for complete, and only implement one way of doing it, but at
least it's a first step.
The idea here is to offer the librarian user the ability to
enable/disable 2FA when logging in to Koha.

It will use time-based, one-time passwords (TOTP) as the second factor,
an application to handle that will be required.

https://en.wikipedia.org/wiki/Time-based_One-Time_Password

More developements are possible on top of this:
* Send a notice (sms or email) with the code
* Force 2FA for librarians
* Implementation for OPAC
* WebAuthn, FIDO2, etc. - https://fidoalliance.org/category/intro-fido/

Test plan:
 0.
  a. % apt install -y libauth-googleauth-perl && updatedatabase && restart_all
  b. To test this you will need an app to generate the TOTP token, you can
 use FreeOTP that is open source and easy to use.
 1. Turn on TwoFactorAuthentication
 2. Go to your account, click 'More' > 'Manage Two-Factor authentication'
 3. Click Enable, scan the QR code with the app, insert the pin code and
 register
 4. Your account now requires 2FA to login!
 5. Notice that you can browse until you logout
 6. Logout
 7. Enter the credential and the pincode provided by the app
 8. Logout
 9. Enter the credential, no pincode
10. Confirm that you are stuck on the second auth form (ie. you cannot
access other Koha pages)
11. Click logout => First login form
12. Enter the credential and the pincode provided by the app

Sponsored-by: Orex Digital

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-20 20:43:15 -10:00
211044ad1a Bug 30410: Unit tests
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-13 15:55:40 +02:00
61f82da49a Bug 30063: Add selenium tests
Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-04 09:47:01 +02:00
35fb56169a Bug 30063: Fix selenium tests
Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-04 09:47:01 +02:00
8fdfa80609 Bug 30181: (QA follow-up) Update BatchTest
This patch updates t::lib::Koha::BackgroundJob::BatchTest to the new
style, and also removes a couple stray cases in which job_id was still
passed as a parameter.

Tests are rewritten a bit, so they actually test more of the behaviors.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/BackgroundJobs.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-22 10:17:33 -10:00
1bbfda8c78 Bug 30072: (follow-up) Fix UT after Bug 29857
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-03 16:11:50 -10:00
f51868cb82 Bug 30072: Unit tests
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-03 16:11:49 -10:00
2b9fae5445 Bug 29857: Rename base class as Koha::Exception
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-01 22:47:47 -10:00
5520b8fb84 Bug 29458: Fix selenium test
The authentication.t selenium tests (and a couple of others) were
failing with:
Error while executing command: element not interactable: Element <input class="btn btn-primary" type="submit"> could not be scrolled into view at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 411. at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 356.

We changed the other of the form, and t::lib::Selenium::submit_form was
not getting the correct (first) form. The one from the auth modal was
retrieved and submit button was clicked. Selenium raised an error as it
is not displayed.
The ->is_displayed selenium method does not work, as per the doc
"""
Note: This does *not* tell you an element's 'visibility' property; as it still takes up space in the DOM and is therefore considered 'displayed'.
"""
https://metacpan.org/pod/Selenium::Remote::WebElement#is_displayed

"The internet" is saying we should be able to use the following in our
xpath expression: not(ancestor::div[contains(@style,'display:none')]
but it actually only works if the display:none rule is defined on the
node (not from .css). Which does not work for us.

The only solution I found is to check for the size of the element, which
is (0,0) if not effectively displayed.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-02-11 09:11:39 -10:00
20366164af Bug 29495: Fix TestBuilder.t
Value not allowed for auto_incr issue_id in Issue at /kohadevbox/koha/t/lib/TestBuilder.pm line 387.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-01-18 23:03:59 -10:00
53956232f9 Bug 29565: Prevent regressions.t to fail on slow boxes
It's failing randomly on some Jenkins' nodes

 #   Failed test 'Encoding in session variables'
 #   at t/db_dependent/selenium/regressions.t line 300.
 Can't call method "get_text" on an undefined value at t/db_dependent/selenium/regressions.t line 285.

It can be recreated locally with the following changes:
@ t/lib/Selenium.pm:50 @ sub new {
     );
     bless $self, $class;
     $self->add_error_handler;
-    $self->driver->set_implicit_wait_timeout(5000);
+    $self->driver->set_implicit_wait_timeout(1000);
     return $self;
 }

@ t/lib/Selenium.pm:50 @ sub new {
     );
     bless $self, $class;
     $self->add_error_handler;
-    $self->driver->set_implicit_wait_timeout(5000);
+    $self->driver->set_implicit_wait_timeout(1000);
     return $self;
 }

This patch suggests to simply double the timeout.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-11-24 09:38:13 +01:00
22392e4a61 Bug 29273: Remove warnings from plugin tests
This patch is suggesting to move the broken test plugins into a separate
directory.

t/lib/plugins and t/lib/bad_plugins

Signed-off-by: David Nind <david@davidnind.com>
JK: Add executable bit to Broken.t
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-11-15 12:38:39 +01:00
064933165b Bug 27173: Add tests
Test plan:
Run t/db_dependent/Koha/Plugins/authority_hooks.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-11-03 15:40:52 +01:00
b296b8ae23 Bug 29350: Don't need to escape the 'delete' TT methods
Will be needed by bug 22605.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-11-02 15:58:08 +01:00
4988c80905 Bug 29368: Correctly mock Zebra index from tests
This is just reusing what is done in Search.t (and that is correct).
But search_utf8.t and remove_from_cart.t are wrong as we want to use the
UI (and we cannot mock the zebra index, ie. koha-conf, from tests for
plack).

This still needs some work but improve a bit the existing code and make
tests pass (hopefully!)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-29 15:45:53 +02:00
370a79648d Bug 17314: Make TestBuilder set good defaults for Suggestions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 17:32:34 +02:00
54a1c5433b Bug 19185: Fix regressions.t
The following test is failing:
| #   Failed test 'OPAC - Remove from cart'
| #   at t/db_dependent/selenium/regressions.t line 132.
| Can't call method "get_value" on an undefined value at t/db_dependent/selenium/regressions.t line 110.
| # Looks like your test exited with 2 just after 3.
| [12:14:08] t/db_dependent/selenium/regressions.t

We are dependind on the search engine and the records in the DB but the
installer is not inserting any records.

This patch is suggesting to reuse the code from search_utf8
(and so make it reusable first) for remove_from_cart test.
This code is mocking the Zebra index with some MARC data and so the
search will return results. We will finally be able to click on the
add to/remove from cart links.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-26 16:46:02 +02:00
f02a58da9e Bug 19185: Fix language selection
'en' is in an optgroup

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
f22576157f Bug 19185: Use submit button if only one exists on the page
During the installation process we do not have fieldset.action, in order
to not complicate changes we are just going to use the submit button if
only one exists.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
cfd25f0ed7 Bug 19821: Require database_test != database
To prevent developpers to drop their database.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
98c42b8d5a Bug 19821: Do not modify KOHA_CONF, use new database_test entry instead
I have tried to use the original idea of this patch for bug 19185, but
it cannot work as it. ENV is not shared between processus and KOHA_CONF
will not be read by webserver (used for selenium tests).

Moreover on bug 19185 we are testing the installation process and we do
not want the database to be populated with sample data, we just need a
new and clean DB.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
Julian Maurice
d33552f871 Bug 19821: Install sample data, ES mappings and Version syspref
Also move .proverc to .proverc.dist, so it cannot be used by accident.

Different ways to use it:

1/ prove --rc .proverc.dist
   Use defaults

2/ cp .proverc.dist .proverc && prove
   Allows to configure db name, marcflavour, and prove options

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
Julian Maurice
a78c4b45e4 Bug 19821: Close filehandle after writing
Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
Julian Maurice
0308261de6 Bug 19821: Run tests on a separate database
Running the tests on the same database as the one used for dev has some
drawbacks:
- Everybody has different data, so we cannot make any assumptions about
data in tests and it can make tests fail for non-obvious reasons.
- Tests have to clean up every change to the database using SQL
transactions, so we cannot write testable code that use transactions
(AFAIK)
- Transactions in tests happen to be committed sometimes, resulting in
garbage data added to the dev database

This patch provides a .proverc file that will load t/lib/Bootstrap.pm
before the tests are run.
t/lib/Bootstrap.pm is responsible for recreating a fresh database and
telling the test scripts to use it.
To use it, just run prove normally from the root directory of Koha.
By default, the database is 'koha_test' and it's created using MARC21
SQL files, it can be changed by running:

    prove --norc \
    --Mt::lib::Bootstrap=database,koha_test,marcflavour,UNIMARC

Test plan:
1. Apply bug 19185 which will also apply these patches
2. In the DBMS run `select count(*) from koha_kohadev.borrowers;`
     (adapt if your usual koha DB isn't koha_kohadev)
3. Run the test plan of bug 19185 and during execution of the
   installation test, pay attention to the following:
4. List the databases in the DBMS (show databases;) to ensure that
   koha_test is created
5. After the test has run, the koha_test database should not be here anymore.
6. In the DBMS run `select count(*) from koha_kohadev.borrowers;`
7. That was to verify that the database you use usually was untouched.
   The counts should be the same.

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 11:27:40 +02:00
ddef2c6a0d Bug 28211: Fix test
#   Failed test 'Plugin enabled, route defined'
    #   at t/db_dependent/Koha/REST/Plugin/PluginRoutes.t line 125.
    # Looks like you failed 1 test of 2.

105         $good_plugin = $plugin
106             if $plugin->{metadata}->{description} eq 'Test plugin';

The wrong plugin was considered the good one.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-19 09:25:25 +02:00
71914b14d1 Bug 28211: (QA follow-up) Add unit tests
JD amended patch: typo multipled ==> multiplied

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-18 11:28:41 +02:00
6cf8bcef44 Bug 28211: (QA follow-up) Remove test_call_recursive
Obsoleted now.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-18 11:28:41 +02:00
18c1500772 Bug 28211: Replace use of call_recursive() with call()
This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-18 11:28:41 +02:00
ec3bce4fa1 Bug 26352: (QA follow-up) Add additional tests
This patch adds additional unit tests for autoMemberNum handling and
corrects the implimentation there.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 14:56:09 +02:00
b0b2554d5f Bug 26352: Add unit test
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 14:56:09 +02:00
b2b1483f4d Bug 26351: Add plugin hooks to transform item barcodes
Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
   and anywhere else you can scan an item barcode, type in some
   valid barcodes, but prefix them with X and postfix them with
   Y, e.g. X123456Y
5) Note the letters are removed by Koha!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Fix QA script issue

* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Remove unused method barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: (QA follow-up) Fix Checkouts.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: Use call_recursive() as a replacement for call()

The method `call()` is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. The `call_recursive()` method takes the output of the first
plugin and uses it as the input for the next plugin and so on. This allowes
each plugin to see the current version of the barcode and modify it if
necessary.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 26351: Fix t/db_dependent/Koha/Plugins/Circulation_hooks.t

Bug 26351: Revert improper change to unit test, fix number of tests

Bug 26351: Remove uneeded use Koha::Plugins statements

Left over from previous changes

Bug 26351: Add missing barcodedecode import

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 14:56:09 +02:00
fc68d49897 Bug 28026: Add call_recursive() as a supplement for call()
The method call() is not sufficient for all plugin hook types. It's
possible that more than one plugin will be installed that wants to
return the arguaments passed in an updated form.  These transformation
plugins need to work recursively rather than independantly.

This patch adss a `call_recursive()` method that takes the output of
the first plugin and uses it as the input for the next plugin and so
on. This allowes each plugin to see the current version of the arguament
list and modify it as necessary.

Test plan
1/ Run the included tests - t/db_dependent/Koha/Plugins/Plugins.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 14:56:09 +02:00
edfa6b9ffe Bug 29121: Add POD to plugins
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-01 16:28:55 +02:00
edc8f49335 Bug 29121: Regression tests
This patch adds regression tests for broken ->install and ->upgrade
methods on plugins.

It adds two dummie plugins named BrokenInstall and BrokenUpgrade, for
convenience.

Tests are added to catch the warnings that will be logged when trying to
load this plugins on different scenarios.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-01 16:28:55 +02:00
2ae5204bfe Bug 27155: Include identifier test in Biblio_and_Items_plugin_hooks.t
The biblio and item action hooks pass a biblionumber or itemnumber now
to the plugin at time of deletion.
This patch adds a tiny refinement to Biblio_and_Items_plugin_hooks.t
and the associated test plugin to be sure of that.

Test plan:
Run t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-09-29 11:34:20 +02:00
d587341d34 Bug 17600: Remove wrong C4::Context imports
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-09-20 14:44:27 +02:00
1c115fcff7 Bug 28306: Switch back to using 'mock' instead of 'redefine'
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-31 10:27:52 +02:00
Julian Maurice
4605304f62 Bug 28306: Allow to query database with minimal memory footprint
The goal is to be able to build a database handler (dbh) and to execute
queries without loading unnecessary stuff. This will be useful to reduce
memory usage of daemons that need to check the database
periodically

The patch provides a new method Koha::Database::dbh which returns a
database handler without loading the DBIx::Class schema. This method is
also used by DBIx::Class, so whether you use DBI or DBIx::Class, the
same method is used to initialize the connection.

The patch also moves some code in order to avoid loading C4::Context:
- C4::Context::timezone moves to Koha::Config
- C4::Context::db_scheme2dbi moves to Koha::Database

To measure memory usage I used the following commands:

* before the patch:
perl -MKoha::Database \
    -E 'Koha::Database->schema->storage->dbh->do("select 1");' \
    -E '$|=1; say $$; sleep 2' \
    | while read pid; do ps -p $pid -o rss=; done

* after the patch:
perl -MKoha::Database \
    -E 'Koha::Database->dbh->do("select 1");' \
    -E '$|=1; say $$; sleep 2' \
    | while read pid; do ps -p $pid -o rss=; done

It will give you the RSS (Resident Set Size) of the perl process in kB

What I get:
* before the patch: between 96.9MB and 97.2MB
* after the patch: between 17.8MB and 18.2MB

Note that if a timezone is configured (either from $KOHA_CONF or
TZ environment variable), Koha will load DateTime::Timezone to check if
it's valid, and it increases RSS to 36MB

Another interesting metric is the number of modules loaded:
* before the patch:
perl -MKoha::Database \
    -E 'Koha::Database->schema->storage->dbh;' \
    -E 'say scalar keys %INC'

Result: 567

* after the patch:
perl -MKoha::Database \
    -E 'Koha::Database->dbh;' \
    -E 'say scalar keys %INC'

Result: 51

Test plan:
1. Apply the patch & restart starman
2. Make sure Koha is still ok (ie. can access the database, does not
have encoding issues, ...)
3. Run the tests in t/Context.t, t/Koha/Config.t,
t/db_dependent/Koha/Database.t, t/timezones.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-31 10:27:52 +02:00
9d6d641d1f Bug 17600: Standardize our EXPORT_OK
On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.

That way we will need to explicitely define the subroutine we want to
use from a module.

This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests

And a lot of other manual changes.

export.pl is a dirty script that can be found on bug 17600.

"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;

The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules

Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).

EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.

@EXPORT and @EXPORT_OK are the two main variables used during export operation.

@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.

@EXPORT_OK does export of symbols on demand basis.
"""

If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
  - use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-07-16 08:58:47 +02:00
ba13ce40f3 Bug 28636: Add POD to t:lib::Mocks
There's no POD. This patch adds it. I wrote it as part of bug 28615 but
then it took another direction, so submitting here.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-07-12 15:47:34 +02:00
d5a792ddf5 Bug 28615: (follow-up) Disable strict mode explicitly
So Debian 9's version of Test::MockModule doens't have ->redefine, and
Ubuntu 20.04's doesn't recognise qw(nostrict). So the only solution is
to just remove the keywords use completely and move back to using
->mock, as the rest of the codebase.

FIXME: using ->mock might be hiding some errors (like a method not being
defined/removed) and should be avoided. ->redefine will explode if the
method doesn't already exist, which is what we want, to catch this kind
of errors. That's why ->mock in strict mode is forbidden. We should try
packaging a newer Test::MockModule ourselves.

Tested on master-buster, master-stretch and master-focal.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2021-06-29 12:24:55 -03:00
ec382b926c Bug 28615: (follow-up) Newer Test::MockModule forbid ->mock
In strict mode, ->mock is forbidden and ->redefine needs to be used
instead. I tested this on buster to see if it breaks something, but it
doesn't.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2021-06-29 11:18:14 -03:00
5754411d66 Bug 28615: Add a simple way to mock Koha::Logger
This patch introduces a new way to mock and test Koha::Logger.

As the POD says, it is used by calling

    my $logger = t::lib::Mocks::Logger->new();

It then provides convenient methods for testing the logging itself per
log-level:

* warn_is
* warn_like
* debug_is
* debug_like
...

Methods for counting the logging activity and also for clearing the mock
buffer are provided as well. This is covered in the POD and also on the
follow-up, that makes use of this to fix Auth_with_shibboleth.t

To test:

1. Run:
   $ kshell
  k$ prove t/Auth_with_shibboleth.t
=> FAIL: Tests fail! It expects some warns but they are not returned by
the lib
2. Apply this patches
3. Repeat 1
=> SUCCESS: Tests pass! The tests now use the new lib, and they
correctly find the logging Auth_with_shibboleth.pm does on function
calls.
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2021-06-29 10:01:02 -03:00
fbecb78ca9 Bug 28516: Prevent failures if borrower_attribute_types.mandatory=1
TestBuilder will generate an integer for the
Koha::Patron::Attribute::Type object, but if 1 is picked some tests are
failing randomly
At least t/db_dependent/Koha/Patrons.t and t/db_dependent/Koha/Patrons/Import.t
The expection "Missing mandatory extended attribute" is raised when the
patron is stored.

Test plan:
The following script should return 0 when the patch is applied:

"""
use t::lib::TestBuilder;
my $builder = t::lib::TestBuilder->new;
my $x = $builder->build_object(
    {
        class => 'Koha::Patron::Attribute::Types',
    }
);
say $x->mandatory;
"""

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-23 14:56:53 +02:00
Joonas Kylmälä
399b24d537 Bug 28479: Use primary keys to check object existence in TestBuilder
The TestBuilder::build_object function used any foreign keys to check
whether an object already exists or not. This brought incorrectly
results of unrelated objects because using any other keys other than
primary keys don't guarantee our results to point to one single
object. For example, as is put here in the unit test, if you created
two items with the same biblionumber and then tried to create a hold
using build_object() we were using the biblionumber to check whether
an item was linked to the hold already. Thus, we were checking whether
a random item was already linked to the hold instead of the one we
wanted either by passing it explicitly to build_object() or the one
build_object() created implicitly. This also resulted in following
warnings when there were more than one match:

DBIx::Class::Storage::DBI::select_single(): Query returned more than
one row.  SQL that returns multiple rows is DEPRECATED for ->find and
->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235

To test:
 $ prove t/db_dependent

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-21 12:07:28 +02:00
a4da4b7568 Bug 28250: Remove upload of debug for Selenium failures
We added an upload of the source page and of the screenshot but both services are discontinued.

If we need them back we must host them ourselves.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-30 17:07:31 +02:00
e751ce8cf2 Bug 28249: Prevent Selenium->wait_for_element_visible to fall in an infinite loop
Stop if max_retries is reached

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-30 17:07:31 +02:00
8fce3401e7 Bug 28234: TestBuilder->build_sample_biblio should set UTF-8 encoding for record
This patch makes the build_sample_biblio method, correctly set the UTF-8
flag for the MARC::Record object.

Tests are added.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/TestBuilder.t
FAIL: Tests fail! An unexpected encoding warning shows
3. Apply this patch
4. Repeat 2
=> SUCCESS: No warning! Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-28 10:34:36 +02:00
James O'Keeffe
2b607d8414 Bug 24000: Some modules do not return 1
This patch adds "1;" to the end of the following perl files:
Koha/Filter/MARC/EmbedItemsAvailability.pm
Koha/Filter/MARC/EmbedSeeFromHeadings.pm
Koha/Filter/MARC/Null.pm
Koha/Item/Search/Field.pm
Koha/SearchEngine.pm
misc/translator/VerboseWarnings.pm
t/lib/Koha/Plugin/Test.pm

This indicates the succesful execution of the initialization code.

Test plan:
Ensure that there are no other perl files that need "1;", but dont have
it.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-06 15:56:30 +02:00
6be845a785 Bug 27336: Fix xt/find-missing-filters.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-01-11 10:27:37 +01:00