Bug 18445 - Tests in t stumble over Auth's BEGIN block
Summary: Tests in t stumble over Auth's BEGIN block
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Galen Charlton
URL:
Keywords:
Depends on: 18420
Blocks:
  Show dependency treegraph
 
Reported: 2017-04-18 12:51 UTC by Marcel de Rooy
Modified: 2023-06-08 22:26 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18445: Replace two use C4::Auth statements (2.70 KB, patch)
2017-04-18 13:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18445: Fix Log.t by mocking HoldsLog (947 bytes, patch)
2017-04-18 13:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18445: Resolve a warning in SMS.t by mocking config (1001 bytes, patch)
2017-04-18 13:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18445: Remove Test::DBIx::Class from XSLT.t (3.22 KB, patch)
2017-04-18 14:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-04-18 12:51:26 UTC
We should be able to run the tests in t without the database.
But if some module loads Auth.pm via C4/Serials or Koha/Virtualshelf directly or indirectly, this will trigger execution of Auth's BEGIN block. This block contains three calls for systempreferences (one from Auth_with_cas). Note that loading Biblio.pm also starts a chain going to Auth.pm.

This report changes two use C4::Auth statements into requires. This will prevent or postpone execution of this code. Note also that this will resolve a few failing tests like (Matcher.t and Prices.t) where the schema mock actually was too late and the database was accessed instead of defined fixtures.

Along the way we fix a few minor issues in t where database access should be eliminated.
Comment 1 Marcel de Rooy 2017-04-18 13:28:33 UTC
Created attachment 62290 [details] [review]
Bug 18445: Replace two use C4::Auth statements

Both Serials.pm and Virtualshelf.pm only need haspermission in a specific
sub. Since loading the module at compile-time triggers early execution of
the BEGIN block, this results in database access in the tests that are
assumed to be database independent.

At the same time we are resolving test failures in Matcher.t and Prices.t
because the schema mock is too late.

Test plan:
[1] Do not yet apply this patch. Backup your database.
[2] Drop the database and create a new empty one.
[3] Run the tests in t. Look at Matcher.t and Prices.t too.
    You will see quite some occurrences of:
    DBD::mysql::st execute failed: Table 'koha_master.systempreferences' doesn't exist [for Statement "SELECT `me`.`variable`, `me`.`value`, `me`.`options`, `me`.`explanation`, `me`.`type` FROM `systempreferences` `me` WHERE ( `me`.`variable` = ? )" with ParamValues: 0='casauthentication'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832.

[4] Apply this patch.
[5] Run the tests in t again. Check the results of Matcher.t and Prices.t.
[6] Restore your database.
[7] Run t/db_dependent/Serials_2.t to trigger the change in Serials.
[8] Run t/db_dependent/Virtualshelves.t to test Koha/Virtualshelf.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2017-04-18 13:34:38 UTC
Created attachment 62292 [details] [review]
Bug 18445: Fix Log.t by mocking HoldsLog

The HoldsLog preference was added later on (bug 14642).
It should be mocked too in this test.

Test plan:
Run t/Log.t (with an empty Koha database).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2017-04-18 13:39:44 UTC
Created attachment 62293 [details] [review]
Bug 18445: Resolve a warning in SMS.t by mocking config

Resolves the warning:
  "Use of uninitialized value in subroutine entry at C4/SMS.pm line 98"
when sms_send_config does not exist in your koha-conf.xml.

Test plan:
Run SMS.t without and with this patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2017-04-18 14:57:43 UTC
Created attachment 62299 [details] [review]
Bug 18445: Remove Test::DBIx::Class from XSLT.t

Removing the tests around get_xslt_sysprefs from XSLT.t.
It makes no sense to test sysprefs values in a test that should be
database agnostic.

Will add t/db_dependent/XSLT.t to rescue the ideas behind these lines.

Test plan:
Run t/XSLT.t
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Jonathan Druart 2017-04-20 17:51:19 UTC
Marcel, did you see bug 18226 and related?
I don't think we should replace the use with require to make the tests pass.
Comment 6 Marcel de Rooy 2017-04-21 14:07:30 UTC
(In reply to Jonathan Druart from comment #5)
> Marcel, did you see bug 18226 and related?
> I don't think we should replace the use with require to make the tests pass.

Ah, no not yet. Will be looking.
Comment 7 Marcel de Rooy 2017-05-16 11:59:49 UTC
Returning here later