Bug 20635 - Write authentication tests with Test::Mojo
Summary: Write authentication tests with Test::Mojo
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on: 20582
Blocks:
  Show dependency treegraph
 
Reported: 2018-04-23 05:56 UTC by Julian Maurice
Modified: 2019-07-02 08:32 UTC (History)
3 users (show)

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


Attachments
Bug 20635: Write authentication tests with Test::Mojo (6.38 KB, patch)
2018-04-23 05:57 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20635: Make sure CGI::Compile is loaded first in tests (1.19 KB, patch)
2019-07-02 08:31 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2018-04-23 05:56:33 UTC
The authentication code needs to be rewritten.
There were several attempts (bugs 7174, 18315) and other bugs without patch that talk about it (12547, 12548, 15204).

Bug 13503 says that before rewriting one of the most sensible area of Koha, we need to write tests.
Bug 19181 contain some tests, but they require Selenium, firefox, xfvb, java, and a running web server.

With Mojolicious we can write equivalent tests without all those dependencies.
Comment 1 Julian Maurice 2018-04-23 05:57:23 UTC
Created attachment 74709 [details] [review]
Bug 20635: Write authentication tests with Test::Mojo

The authentication code needs to be rewritten.
There were several attempts (bugs 7174, 18315) and other bugs without
patch that talk about it (12547, 12548, 15204).

Bug 13503 says that before rewriting one of the most sensible area of
Koha, we need to write tests.
Bug 19181 contain some tests, but they require Selenium, firefox, xfvb,
java, and a running web server.

With Mojolicious we can write equivalent tests without all those
dependencies.

This patch add basic tests for password and CAS authentication.

Test plan:
1. Apply bug 20582
2. Apply this patch
3. Run `prove t/integration/auth`
Comment 2 Josef Moravec 2019-01-30 15:32:39 UTC
Just tried, but without any success, not sure what's wrong here:

1..2
# Subtest: intranet
    1..10
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
Reference is already weak at /usr/local/lib/x86_64-linux-gnu/perl/5.24.1/Template/Plugin/Filter.pm line 68.
A context appears to have been destroyed without first calling release().
Based on $@ it does not look like an exception was thrown (this is not always
a reliable test)

This is a problem because the global error variables ($!, $@, and $?) will
not be restored. In addition some release callbacks will not work properly from
inside a DESTROY method.

Here are the context creation details, just in case a tool forgot to call
release():
  File: t/integration/auth/password.t
  Line: 58
  Tool: Test::More::subtest

Cleaning up the CONTEXT stack...
# Test ended with extra hubs on the stack!
    # No tests run!
Comment 3 Mark Tompsett 2019-07-02 05:16:25 UTC
prove -v t/integration/auth/
t/integration/auth/cas.t ....... 
1..2
# Subtest: intranet
    1..10
[Tue Jul  2 05:13:43 2019] [debug] GET "/cgi-bin/koha/mainpage.pl"
[Tue Jul  2 05:13:43 2019] [debug] Routing to a callback
A context appears to have been destroyed without first calling release().
Based on $@ it does not look like an exception was thrown (this is not always
a reliable test)

This is a problem because the global error variables ($!, $@, and $?) will
not be restored. In addition some release callbacks will not work properly from
inside a DESTROY method.

Here are the context creation details, just in case a tool forgot to call
release():
  File: t/integration/auth/cas.t
  Line: 74
  Tool: Test::More::subtest

Cleaning up the CONTEXT stack...
# Test ended with extra hubs on the stack!
    # No tests run!
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 2/2 subtests 
t/integration/auth/password.t .. 
1..2
# Subtest: intranet
    1..10
[Tue Jul  2 05:13:46 2019] [debug] GET "/cgi-bin/koha/mainpage.pl"
[Tue Jul  2 05:13:46 2019] [debug] Routing to a callback
A context appears to have been destroyed without first calling release().
Based on $@ it does not look like an exception was thrown (this is not always
a reliable test)

This is a problem because the global error variables ($!, $@, and $?) will
not be restored. In addition some release callbacks will not work properly from
inside a DESTROY method.

Here are the context creation details, just in case a tool forgot to call
release():
  File: t/integration/auth/password.t
  Line: 58
  Tool: Test::More::subtest

Cleaning up the CONTEXT stack...
# Test ended with extra hubs on the stack!
    # No tests run!
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 2/2 subtests 

Test Summary Report
-------------------
t/integration/auth/cas.t     (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 2 tests but ran 0.
t/integration/auth/password.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 2 tests but ran 0.
Files=2, Tests=0,  5 wallclock secs ( 0.02 usr  0.00 sys +  4.16 cusr  0.59 csys =  4.77 CPU)
Result: FAIL


-- confirmed the same problem in comment #2, marking Failed QA. Though, I don't know if it is this patch, or the one it depends on, that is the problem.
Comment 4 Julian Maurice 2019-07-02 07:51:36 UTC
This has something to do with the safe_exit subroutine in C4::Auth, called by checkauth, which calls exit because psgi_env returns false. %ENV does not contain any 'psgi.' variables, but I don't know why...
Comment 5 Julian Maurice 2019-07-02 08:31:06 UTC
Created attachment 91176 [details] [review]
Bug 20635: Make sure CGI::Compile is loaded first in tests

CGI::Compile needs to be loaded before Koha modules for it to work
normally.
Even if not directly needed in test files, we need to 'use' it
explicitely so that it loads before Koha modules.
Comment 6 Julian Maurice 2019-07-02 08:32:44 UTC
The last patch fixes the problem mentioned in comment 2 and comment 3, and make t/integration/auth/password.t pass.
However the CAS test still fails, for other reasons.