From d6ede65026bb183aae1c751c8b067f3536a34039 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 6 Nov 2015 15:12:25 +0000
Subject: [PATCH] [SIGNED OFF] Bug 15152: Try and make Guided Reports related
 tests pass

Jenkins is currently failing on

t_db_dependent_Reports_Guided_t.filter takes report id (393)
Failing for the past 1 build (Since Unstable#190 )
Took 2 ms.
Error Message

not ok 8 - filter takes report id

Standard Output

1..19
ok 1 - use C4::Reports::Guided;
ok 2 - Not breaking subqueries
ok 3 - Returns correct default offset
ok 4 - Returns correct default LIMIT
ok 5 - Correctly removes only final LIMIT
ok 6 - Returns correct default offset
ok 7 - Returns correct extracted LIMIT
ok 8 - Correctly removes only final LIMIT (with offset)
ok 9 - Returns correct extracted offset
ok 10 - Returns correct extracted LIMIT
ok 11 - Not breaking simple queries
ok 12 - Returns correct default offset
ok 13 - Returns correct default LIMIT
ok 14 - Correctly removes LIMIT in simple case
ok 15 - Returns correct default offset
ok 16 - Returns correct extracted LIMIT
ok 17 - Correctly removes LIMIT in simple case (with offset)
ok 18 - Returns correct extracted offset
ok 19 - Returns correct extracted LIMIT

Which does not make sense. I suspect that there is a mismatch with the
 different filenames.

This patch groups the different tests for Guided reports into the same
file.
The goal is to make Jenkins happy going all-in with this assumption.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
---
 t/db_dependent/Reports/Guided.t | 318 ++++++++++++++++++++++++++++++++--------
 t/db_dependent/ReportsGuided.t  | 102 -------------
 t/db_dependent/Reports_Guided.t | 121 ---------------
 3 files changed, 255 insertions(+), 286 deletions(-)
 delete mode 100755 t/db_dependent/ReportsGuided.t
 delete mode 100755 t/db_dependent/Reports_Guided.t

diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t
index 506acee..f369d77 100644
--- a/t/db_dependent/Reports/Guided.t
+++ b/t/db_dependent/Reports/Guided.t
@@ -1,4 +1,5 @@
 # Copyright 2012 Catalyst IT Ltd.
+# Copyright Koha Development Team
 #
 # This file is part of Koha.
 #
@@ -15,75 +16,266 @@
 # You should have received a copy of the GNU General Public License
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
-use strict;
-use warnings;
+use Modern::Perl;
 
-use Test::More tests => 19;                      # last test to print
+use Test::More tests => 4;
+use Test::MockModule;
+use Test::Warn;
 
 use_ok('C4::Reports::Guided');
 
-# This is the query I found that triggered bug 8594.
-my $sql = "SELECT aqorders.ordernumber, biblio.title, biblio.biblionumber, items.homebranch,
-    aqorders.entrydate, aqorders.datereceived,
-    (SELECT DATE(datetime) FROM statistics
-        WHERE itemnumber=items.itemnumber AND
-            (type='return' OR type='issue') LIMIT 1)
-    AS shelvedate,
-    DATEDIFF(COALESCE(
+subtest 'prevent from breaking some subqueries' => sub {
+    plan tests => 18;
+    # This is the query I found that triggered bug 8594.
+    my $sql = "SELECT aqorders.ordernumber, biblio.title, biblio.biblionumber, items.homebranch,
+        aqorders.entrydate, aqorders.datereceived,
         (SELECT DATE(datetime) FROM statistics
             WHERE itemnumber=items.itemnumber AND
-            (type='return' OR type='issue') LIMIT 1),
-    aqorders.datereceived), aqorders.entrydate) AS totaldays
-FROM aqorders
-LEFT JOIN biblio USING (biblionumber)
-LEFT JOIN items ON (items.biblionumber = biblio.biblionumber
-    AND dateaccessioned=aqorders.datereceived)
-WHERE (entrydate >= '2011-01-01' AND (datereceived < '2011-02-01' OR datereceived IS NULL))
-    AND items.homebranch LIKE 'INFO'
-ORDER BY title";
-
-my ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($sql);
-is($res_sql, $sql, "Not breaking subqueries");
-is($res_lim1, 0, "Returns correct default offset");
-is($res_lim2, undef, "Returns correct default LIMIT");
-
-# Now the same thing, but we want it to remove the LIMIT from the end
-
-my $test_sql = $res_sql . " LIMIT 242";
-($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
-# The replacement drops a ' ' where the limit was
-is(trim($res_sql), $sql, "Correctly removes only final LIMIT");
-is($res_lim1, 0, "Returns correct default offset");
-is($res_lim2, 242, "Returns correct extracted LIMIT");
-
-$test_sql = $res_sql . " LIMIT 13,242";
-($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
-# The replacement drops a ' ' where the limit was
-is(trim($res_sql), $sql, "Correctly removes only final LIMIT (with offset)");
-is($res_lim1, 13, "Returns correct extracted offset");
-is($res_lim2, 242, "Returns correct extracted LIMIT");
-
-# After here is the simpler case, where there isn't a WHERE clause to worry
-# about.
-
-# First case with nothing to change
-$sql = "SELECT * FROM items";
-($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($sql);
-is($res_sql, $sql, "Not breaking simple queries");
-is($res_lim1, 0, "Returns correct default offset");
-is($res_lim2, undef, "Returns correct default LIMIT");
-
-$test_sql = $sql . " LIMIT 242";
-($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
-is(trim($res_sql), $sql, "Correctly removes LIMIT in simple case");
-is($res_lim1, 0, "Returns correct default offset");
-is($res_lim2, 242, "Returns correct extracted LIMIT");
-
-$test_sql = $sql . " LIMIT 13,242";
-($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
-is(trim($res_sql), $sql, "Correctly removes LIMIT in simple case (with offset)");
-is($res_lim1, 13, "Returns correct extracted offset");
-is($res_lim2, 242, "Returns correct extracted LIMIT");
+                (type='return' OR type='issue') LIMIT 1)
+        AS shelvedate,
+        DATEDIFF(COALESCE(
+            (SELECT DATE(datetime) FROM statistics
+                WHERE itemnumber=items.itemnumber AND
+                (type='return' OR type='issue') LIMIT 1),
+        aqorders.datereceived), aqorders.entrydate) AS totaldays
+    FROM aqorders
+    LEFT JOIN biblio USING (biblionumber)
+    LEFT JOIN items ON (items.biblionumber = biblio.biblionumber
+        AND dateaccessioned=aqorders.datereceived)
+    WHERE (entrydate >= '2011-01-01' AND (datereceived < '2011-02-01' OR datereceived IS NULL))
+        AND items.homebranch LIKE 'INFO'
+    ORDER BY title";
+
+    my ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($sql);
+    is($res_sql, $sql, "Not breaking subqueries");
+    is($res_lim1, 0, "Returns correct default offset");
+    is($res_lim2, undef, "Returns correct default LIMIT");
+
+    # Now the same thing, but we want it to remove the LIMIT from the end
+
+    my $test_sql = $res_sql . " LIMIT 242";
+    ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
+    # The replacement drops a ' ' where the limit was
+    is(trim($res_sql), $sql, "Correctly removes only final LIMIT");
+    is($res_lim1, 0, "Returns correct default offset");
+    is($res_lim2, 242, "Returns correct extracted LIMIT");
+
+    $test_sql = $res_sql . " LIMIT 13,242";
+    ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
+    # The replacement drops a ' ' where the limit was
+    is(trim($res_sql), $sql, "Correctly removes only final LIMIT (with offset)");
+    is($res_lim1, 13, "Returns correct extracted offset");
+    is($res_lim2, 242, "Returns correct extracted LIMIT");
+
+    # After here is the simpler case, where there isn't a WHERE clause to worry
+    # about.
+
+    # First case with nothing to change
+    $sql = "SELECT * FROM items";
+    ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($sql);
+    is($res_sql, $sql, "Not breaking simple queries");
+    is($res_lim1, 0, "Returns correct default offset");
+    is($res_lim2, undef, "Returns correct default LIMIT");
+
+    $test_sql = $sql . " LIMIT 242";
+    ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
+    is(trim($res_sql), $sql, "Correctly removes LIMIT in simple case");
+    is($res_lim1, 0, "Returns correct default offset");
+    is($res_lim2, 242, "Returns correct extracted LIMIT");
+
+    $test_sql = $sql . " LIMIT 13,242";
+    ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql);
+    is(trim($res_sql), $sql, "Correctly removes LIMIT in simple case (with offset)");
+    is($res_lim1, 13, "Returns correct extracted offset");
+    is($res_lim2, 242, "Returns correct extracted LIMIT");
+};
+
+subtest "Test save_report and delete_report" => sub {
+    plan tests => 12;
+    my $context = new Test::MockModule('C4::Context');
+    my $koha = new Test::MockModule('C4::Koha');
+
+    sub MockedIsAuthorisedValueCategory {
+        my $authorised_value = shift;
+
+        if ( $authorised_value eq 'LOC' ) {
+            return 1;
+        } else {
+            return 0;
+        }
+    }
+
+    $koha->mock(
+        'IsAuthorisedValueCategory',
+        \&MockedIsAuthorisedValueCategory
+    );
+
+    {   # GetReservedAuthorisedValues tests
+        # This one will catch new reserved words not added
+        # to GetReservedAuthorisedValues
+        my %test_authval = (
+            'date' => 1,
+            'branches' => 1,
+            'itemtypes' => 1,
+            'cn_source' => 1,
+            'categorycode' => 1,
+            'biblio_framework' => 1,
+        );
+
+        my $reserved_authorised_values = GetReservedAuthorisedValues();
+        is_deeply(\%test_authval, $reserved_authorised_values,
+                    'GetReservedAuthorisedValues returns a fixed list');
+    }
+
+    {
+        ok( IsAuthorisedValueValid('LOC'),
+            'User defined authorised value category is valid');
+
+        ok( ! IsAuthorisedValueValid('XXX'),
+            'Not defined authorised value category is invalid');
+
+        # Loop through the reserved authorised values
+        foreach my $authorised_value ( keys %{GetReservedAuthorisedValues()} ) {
+            ok( IsAuthorisedValueValid($authorised_value),
+                '\''.$authorised_value.'\' is a reserved word, and thus a valid authorised value');
+        }
+    }
+
+    {   # GetParametersFromSQL tests
+
+        my $test_query_1 = "
+            SELECT date_due
+            FROM old_issues
+            WHERE YEAR(timestamp) = <<Year|custom_list>> AND
+                  branchcode = <<Branch|branches>> AND
+                  borrowernumber = <<Borrower>>
+        ";
+
+        my @test_parameters_with_custom_list = (
+            { 'name' => 'Year', 'authval' => 'custom_list' },
+            { 'name' => 'Branch', 'authval' => 'branches' },
+            { 'name' => 'Borrower', 'authval' => undef }
+        );
+
+        is_deeply( GetParametersFromSQL($test_query_1), \@test_parameters_with_custom_list,
+            'SQL params are correctly parsed');
+
+        # ValidateSQLParameters tests
+        my @problematic_parameters = ();
+        push @problematic_parameters, { 'name' => 'Year', 'authval' => 'custom_list' };
+        is_deeply( ValidateSQLParameters( $test_query_1 ),
+                   \@problematic_parameters,
+                   '\'custom_list\' not a valid category' );
+
+        my $test_query_2 = "
+            SELECT date_due
+            FROM old_issues
+            WHERE YEAR(timestamp) = <<Year|date>> AND
+                  branchcode = <<Branch|branches>> AND
+                  borrowernumber = <<Borrower|LOC>>
+        ";
+
+        is_deeply( ValidateSQLParameters( $test_query_2 ),
+            [],
+            'All parameters valid, empty problematic authvals list');
+    }
+};
+
+subtest "Test IsAuthorisedValueValid" => sub {
+    plan tests => 17;
+    can_ok(
+        'C4::Reports::Guided',
+        qw(save_report delete_report execute_query)
+    );
+
+    my $schema = Koha::Database->new->schema;
+    $schema->storage->txn_begin;
+
+    my $dbh = C4::Context->dbh;
+    $dbh->do(q|DELETE FROM saved_sql|);
+    $dbh->do(q|DELETE FROM saved_reports|);
+
+    #Start tests
+
+    #Test save_report
+    my $count = scalar( @{ get_saved_reports() } );
+    is( $count, 0, "There is no report" );
+
+    my @report_ids;
+    for my $id ( 1 .. 3 ) {
+        push @report_ids, save_report({
+            borrowernumber => $id,
+            sql            => "SQL$id",
+            name           => "Name$id",
+            area           => "area$id",
+            group          => "group$id",
+            subgroup       => "subgroup$id",
+            type           => "type$id",
+            notes          => "note$id",
+            cache_expiry   => "null",
+            public         => "null"
+        });
+        $count++;
+    }
+    like( $report_ids[0], '/^\d+$/', "Save_report returns an id for first" );
+    like( $report_ids[1], '/^\d+$/', "Save_report returns an id for second" );
+    like( $report_ids[2], '/^\d+$/', "Save_report returns an id for third" );
+
+    is( scalar( @{ get_saved_reports() } ),
+        $count, "$count reports have been added" );
+
+    is( scalar( @{ get_saved_reports( $report_ids[0] ) } ),
+        1, "filter takes report id" );
+
+    #Test delete_report
+    is (delete_report(),undef, "Without id delete_report returns undef");
+
+    is( delete_report( $report_ids[0] ), 1, "report 1 is deleted" );
+    $count--;
+
+    is( scalar( @{ get_saved_reports() } ), $count, "Report1 has been deleted" );
+
+    is( delete_report( $report_ids[1], $report_ids[2] ), 2, "report 2 and 3 are deleted" );
+    $count -= 2;
+
+    is( scalar( @{ get_saved_reports() } ),
+        $count, "Report2 and report3 have been deleted" );
+
+    my $sth = execute_query('SELECT COUNT(*) FROM systempreferences', 0, 10);
+    my $results = $sth->fetchall_arrayref;
+    is(scalar(@$results), 1, 'running a query returned a result');
+
+    my $version = C4::Context->preference('Version');
+    $sth = execute_query(
+        'SELECT value FROM systempreferences WHERE variable = ?',
+        0,
+        10,
+        [ 'Version' ],
+    );
+    $results = $sth->fetchall_arrayref;
+    is_deeply(
+        $results,
+        [ [ $version ] ],
+        'running a query with a parameter returned the expected result'
+    );
+
+    # for next test, we want to let execute_query capture any SQL errors
+    $dbh->{RaiseError} = 0;
+    my $errors;
+    warning_like { ($sth, $errors) = execute_query(
+            'SELECT surname FRM borrowers',  # error in the query is intentional
+            0, 10 ) }
+            qr/^DBD::mysql::st execute failed: You have an error in your SQL syntax;/,
+            "Wrong SQL syntax raises warning";
+    ok(
+        defined($errors) && exists($errors->{queryerr}),
+        'attempting to run a report with an SQL syntax error returns error message (Bug 12214)'
+    );
+
+    is_deeply( get_report_areas(), [ 'CIRC', 'CAT', 'PAT', 'ACQ', 'ACC', 'SER' ],
+        "get_report_areas returns the correct array of report areas");
+
+};
 
 sub trim {
     my ($s) = @_;
diff --git a/t/db_dependent/ReportsGuided.t b/t/db_dependent/ReportsGuided.t
deleted file mode 100755
index 097722a..0000000
--- a/t/db_dependent/ReportsGuided.t
+++ /dev/null
@@ -1,102 +0,0 @@
-#!/usr/bin/perl
-
-use Modern::Perl;
-
-use Test::More tests => 13;
-use Test::MockModule;
-
-use t::lib::Mocks;
-
-use_ok('C4::Reports::Guided');
-
-my $context = new Test::MockModule('C4::Context');
-my $koha = new Test::MockModule('C4::Koha');
-
-BEGIN {
-    t::lib::Mocks::mock_dbh;
-}
-
-sub MockedIsAuthorisedValueCategory {
-    my $authorised_value = shift;
-
-    if ( $authorised_value eq 'LOC' ) {
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
-$koha->mock(
-    'IsAuthorisedValueCategory',
-    \&MockedIsAuthorisedValueCategory
-);
-
-{   # GetReservedAuthorisedValues tests
-    # This one will catch new reserved words not added
-    # to GetReservedAuthorisedValues
-    my %test_authval = (
-        'date' => 1,
-        'branches' => 1,
-        'itemtypes' => 1,
-        'cn_source' => 1,
-        'categorycode' => 1,
-        'biblio_framework' => 1,
-    );
-
-    my $reserved_authorised_values = GetReservedAuthorisedValues();
-    is_deeply(\%test_authval, $reserved_authorised_values,
-                'GetReservedAuthorisedValues returns a fixed list');
-}
-
-{
-    ok( IsAuthorisedValueValid('LOC'),
-        'User defined authorised value category is valid');
-
-    ok( ! IsAuthorisedValueValid('XXX'),
-        'Not defined authorised value category is invalid');
-
-    # Loop through the reserved authorised values
-    foreach my $authorised_value ( keys %{GetReservedAuthorisedValues()} ) {
-        ok( IsAuthorisedValueValid($authorised_value),
-            '\''.$authorised_value.'\' is a reserved word, and thus a valid authorised value');
-    }
-}
-
-{   # GetParametersFromSQL tests
-
-    my $test_query_1 = "
-        SELECT date_due
-        FROM old_issues
-        WHERE YEAR(timestamp) = <<Year|custom_list>> AND
-              branchcode = <<Branch|branches>> AND
-              borrowernumber = <<Borrower>>
-    ";
-
-    my @test_parameters_with_custom_list = (
-        { 'name' => 'Year', 'authval' => 'custom_list' },
-        { 'name' => 'Branch', 'authval' => 'branches' },
-        { 'name' => 'Borrower', 'authval' => undef }
-    );
-
-    is_deeply( GetParametersFromSQL($test_query_1), \@test_parameters_with_custom_list,
-        'SQL params are correctly parsed');
-
-    # ValidateSQLParameters tests
-    my @problematic_parameters = ();
-    push @problematic_parameters, { 'name' => 'Year', 'authval' => 'custom_list' };
-    is_deeply( ValidateSQLParameters( $test_query_1 ),
-               \@problematic_parameters,
-               '\'custom_list\' not a valid category' );
-
-    my $test_query_2 = "
-        SELECT date_due
-        FROM old_issues
-        WHERE YEAR(timestamp) = <<Year|date>> AND
-              branchcode = <<Branch|branches>> AND
-              borrowernumber = <<Borrower|LOC>>
-    ";
-
-    is_deeply( ValidateSQLParameters( $test_query_2 ),
-        [],
-        'All parameters valid, empty problematic authvals list');
-}
diff --git a/t/db_dependent/Reports_Guided.t b/t/db_dependent/Reports_Guided.t
deleted file mode 100755
index 649e33a..0000000
--- a/t/db_dependent/Reports_Guided.t
+++ /dev/null
@@ -1,121 +0,0 @@
-#!/usr/bin/perl
-
-# This file is part of Koha.
-#
-# Koha is free software; you can redistribute it and/or modify it
-# under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 3 of the License, or
-# (at your option) any later version.
-#
-# Koha is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Koha; if not, see <http://www.gnu.org/licenses>.
-
-use Modern::Perl;
-
-use Test::More tests => 18;
-use Test::Warn;
-
-use C4::Context;
-use Koha::Database;
-
-BEGIN {
-    use_ok('C4::Reports::Guided');
-}
-can_ok(
-    'C4::Reports::Guided',
-    qw(save_report delete_report execute_query)
-);
-
-my $schema = Koha::Database->new->schema;
-$schema->storage->txn_begin;
-
-my $dbh = C4::Context->dbh;
-$dbh->do(q|DELETE FROM saved_sql|);
-$dbh->do(q|DELETE FROM saved_reports|);
-
-#Start tests
-
-#Test save_report
-my $count = scalar( @{ get_saved_reports() } );
-is( $count, 0, "There is no report" );
-
-my @report_ids;
-for my $id ( 1 .. 3 ) {
-    push @report_ids, save_report({
-        borrowernumber => $id,
-        sql            => "SQL$id",
-        name           => "Name$id",
-        area           => "area$id",
-        group          => "group$id",
-        subgroup       => "subgroup$id",
-        type           => "type$id",
-        notes          => "note$id",
-        cache_expiry   => "null",
-        public         => "null"
-    });
-    $count++;
-}
-like( $report_ids[0], '/^\d+$/', "Save_report returns an id for first" );
-like( $report_ids[1], '/^\d+$/', "Save_report returns an id for second" );
-like( $report_ids[2], '/^\d+$/', "Save_report returns an id for third" );
-
-is( scalar( @{ get_saved_reports() } ),
-    $count, "$count reports have been added" );
-
-is( scalar( @{ get_saved_reports( $report_ids[0] ) } ),
-    1, "filter takes report id" );
-
-#Test delete_report
-is (delete_report(),undef, "Without id delete_report returns undef");
-
-is( delete_report( $report_ids[0] ), 1, "report 1 is deleted" );
-$count--;
-
-is( scalar( @{ get_saved_reports() } ), $count, "Report1 has been deleted" );
-
-is( delete_report( $report_ids[1], $report_ids[2] ), 2, "report 2 and 3 are deleted" );
-$count -= 2;
-
-is( scalar( @{ get_saved_reports() } ),
-    $count, "Report2 and report3 have been deleted" );
-
-my $sth = execute_query('SELECT COUNT(*) FROM systempreferences', 0, 10);
-my $results = $sth->fetchall_arrayref;
-is(scalar(@$results), 1, 'running a query returned a result');
-
-my $version = C4::Context->preference('Version');
-$sth = execute_query(
-    'SELECT value FROM systempreferences WHERE variable = ?',
-    0,
-    10,
-    [ 'Version' ],
-);
-$results = $sth->fetchall_arrayref;
-is_deeply(
-    $results,
-    [ [ $version ] ],
-    'running a query with a parameter returned the expected result'
-);
-
-# for next test, we want to let execute_query capture any SQL errors
-$dbh->{RaiseError} = 0;
-my $errors;
-warning_like { ($sth, $errors) = execute_query(
-        'SELECT surname FRM borrowers',  # error in the query is intentional
-        0, 10 ) }
-        qr/^DBD::mysql::st execute failed: You have an error in your SQL syntax;/,
-        "Wrong SQL syntax raises warning";
-ok(
-    defined($errors) && exists($errors->{queryerr}),
-    'attempting to run a report with an SQL syntax error returns error message (Bug 12214)'
-);
-
-is_deeply( get_report_areas(), [ 'CIRC', 'CAT', 'PAT', 'ACQ', 'ACC', 'SER' ],
-    "get_report_areas returns the correct array of report areas");
-
-$schema->storage->txn_rollback;
-- 
2.1.4