Bug 14059 - Fully testing C4::Creators::Lib
Summary: Fully testing C4::Creators::Lib
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Julian FIOL
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-24 12:55 UTC by Julian FIOL
Modified: 2016-12-05 21:22 UTC (History)
5 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 14059 : Testing C4::Creators::Lib (79.12 KB, patch)
2015-04-24 12:57 UTC, Julian FIOL
Details | Diff | Splinter Review
Bug 14059 : Delete all export of get_column_names (1.57 KB, patch)
2015-05-04 09:20 UTC, Julian FIOL
Details | Diff | Splinter Review
Bug 14059 : Testing C4::Creators::Lib (79.17 KB, patch)
2015-05-04 13:10 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14059 : Delete all export of get_column_names (1.67 KB, patch)
2015-05-04 13:11 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14059 : Deleting 'issues' table before processing the tests (803 bytes, patch)
2015-05-05 07:35 UTC, Julian FIOL
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14059: Deleting 'issues' table before processing the tests (893 bytes, patch)
2015-05-07 13:31 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 14059 : Testing C4::Creators::Lib (79.24 KB, patch)
2015-05-22 15:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14059 : Delete all export of get_column_names (1.77 KB, patch)
2015-05-22 15:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14059: Deleting 'issues' table before processing the tests (965 bytes, patch)
2015-05-22 15:09 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!