Bug 14059

Summary: Fully testing C4::Creators::Lib
Product: Koha Reporter: Julian FIOL <julian.fiol>
Component: Test SuiteAssignee: Julian FIOL <julian.fiol>
Status: CLOSED FIXED QA Contact: Galen Charlton <gmcharlt>
Severity: trivial    
Priority: P5 - low CC: bgkriegel, julian.maurice, katrin.fischer, mtompset, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 14059 : Testing C4::Creators::Lib
Bug 14059 : Delete all export of get_column_names
Bug 14059 : Testing C4::Creators::Lib
Bug 14059 : Delete all export of get_column_names
Bug 14059 : Deleting 'issues' table before processing the tests
[SIGNED-OFF] Bug 14059: Deleting 'issues' table before processing the tests
[PASSED QA] Bug 14059 : Testing C4::Creators::Lib
[PASSED QA] Bug 14059 : Delete all export of get_column_names
[PASSED QA] Bug 14059: Deleting 'issues' table before processing the tests

Description Julian FIOL 2015-04-24 12:55:43 UTC
Testing C4::Creators::Lib at 100%
    Deleting get_column_names subroutine (never used)
    
        TEST PLAN
        ---------
        1. Apply patch
        2. prove -v t/db_dependent/Creators/Lib.t
        -- All 644 tests should run successfully without
        any error or warning
    
        TEST PLAN OPTIONAL
        ------------------
    
        Check with bug 13899 to see the coverage of this module.
    
        Coverage BEFORE this patch :
        Statement  :  11,6%
        Branch     :   0,0%
        Condition  :    N/A
        Subroutine :  36,0%
    
        Coverage AFTER this patch :
        Statement  : 100,0%
        Branch     : 100,0%
        Condition  :    N/A
        Subroutine : 100,0%
Comment 1 Julian FIOL 2015-04-24 12:57:52 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2015-05-02 22:13:49 UTC
Comment on attachment 38481 [details] [review]
Bug 14059 : Testing C4::Creators::Lib

Review of attachment 38481 [details] [review]:
-----------------------------------------------------------------

::: C4/Creators/Lib.pm
@@ -448,5 @@
> -Return an arrayref of an array containing the column names of the supplied table.
> -
> -=cut
> -
> -sub get_column_names {

git grep get_column_names shows that C4::Creators exports this function, and this lib still exports it too. Yes, it isn't used, but you forgot to cut the other parts out too.
Comment 3 Mark Tompsett 2015-05-02 22:14:25 UTC
Failed QA'ing this for reason in comment #2.
Comment 4 Mark Tompsett 2015-05-03 06:44:01 UTC
I also had a hard time getting 13899 to run well.
It runs great the first time (long time to run).
But second run after apply doesn't seem to work.
I did a rm -rf cover_db and a reboot to get it to work again after the patch.
This does improve the coverage.
Comment 5 Julian FIOL 2015-05-04 09:20:36 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2015-05-04 13:10:53 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2015-05-04 13:11:01 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2015-05-04 20:20:52 UTC
Can you please make this a little more fail save? I think isses needs to be emptied out in the tests for them to run:

$ prove t/db_dependent/Creators/Lib.t t/db_dependent/Creators/Lib.t .. 1/644 DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha`.`issues`, CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE) at t/db_dependent/Creators/Lib.t line 61.
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha`.`issues`, CONSTRAINT `issues_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON UPDATE CASCADE) at t/db_dependent/Creators/Lib.t line 61.
# Looks like you planned 644 tests but ran 6.
# Looks like your test exited with 255 just after 6.
t/db_dependent/Creators/Lib.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Comment 9 Julian FIOL 2015-05-05 07:35:20 UTC Comment hidden (obsolete)
Comment 10 Mark Tompsett 2015-05-06 03:15:47 UTC
Patch added by comment #9 needs sign off.
Comment 11 Bernardo Gonzalez Kriegel 2015-05-07 13:31:26 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2015-05-22 15:09:00 UTC
Created attachment 39426 [details] [review]
[PASSED QA] Bug 14059 : Testing C4::Creators::Lib

Testing C4::Creators::Lib at 100%
Deleting get_column_names subroutine (never used)

    TEST PLAN
    ---------
    1. Apply patch
    2. prove -v t/db_dependent/Creators/Lib.t
    -- All 644 tests should run successfully without
    any error or warning

    TEST PLAN OPTIONAL
    ------------------

    Check with bug 13899 to see the coverage of this module.

    Coverage BEFORE this patch :
    Statement  :  11,6%
    Branch     :   0,0%
    Condition  :    N/A
    Subroutine :  36,0%

    Coverage AFTER this patch :
    Statement  : 100,0%
    Branch     : 100,0%
    Condition  :    N/A
    Subroutine : 100,0%

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Kyle M Hall 2015-05-22 15:09:08 UTC
Created attachment 39427 [details] [review]
[PASSED QA] Bug 14059 : Delete all export of get_column_names

This deals with my concerns raised in comment #2.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Kyle M Hall 2015-05-22 15:09:10 UTC
Created attachment 39428 [details] [review]
[PASSED QA] Bug 14059: Deleting 'issues' table before processing the tests

It's suppose to avoid foreign key trouble

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No error

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Tomás Cohen Arazi 2015-05-22 20:10:59 UTC
Patches pushed to master.

Good job Julian!