Bug 14147 - Add unit tests to C4::External::OverDrive
Summary: Add unit tests to C4::External::OverDrive
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Julian FIOL
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-05 10:35 UTC by Julian FIOL
Modified: 2015-12-03 22:11 UTC (History)
4 users (show)

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

Attachments
Bug 14147 : Add unit tests to C4::External::OverDrive (3.31 KB, patch)
2015-05-05 10:37 UTC, Julian FIOL 		Details | Diff | Splinter Review
Bug 14147 : Add unit tests to C4::External::OverDrive (3.32 KB, patch)
2015-05-05 12:00 UTC, Julian FIOL 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14147: Add unit tests to C4::External::OverDrive (3.43 KB, patch)
2015-05-07 13:56 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[PASSED QA] Bug 14147: Add unit tests to C4::External::OverDrive (3.48 KB, patch)
2015-05-08 14:22 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Julian FIOL 2015-05-05 10:35:02 UTC 
This patch test 'IsOverDriveEnabled' subroutine
    
        TEST PLAN
        ---------
        1. Apply patch
        2. prove -v t/External/OverDrive.t
        -- All 6 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  :  47,5%
        Branch     :   0,0%
        Condition  :   0,0%
        Subroutine :  75,0%
    
        Coverage AFTER this patch :
        Statement  :  49,1%
        Branch     :   0,0%
        Condition  :  33,3%
        Subroutine :  83,3%
Comment 1 Julian FIOL 2015-05-05 10:37:25 UTC Comment hidden (obsolete)
Comment 2 Julian FIOL 2015-05-05 12:00:34 UTC Comment hidden (obsolete)
Comment 3 Bernardo Gonzalez Kriegel 2015-05-07 13:56:26 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2015-05-08 08:38:12 UTC 
Hey Julian,
I am not sure about passing QA on this patch.
You only actually do some trivial tests for IsOverDriveEnabled.
The real stuff with requests, UserAgent etc. (probably mocking OverDrive behavior somehow) is not there.
Comment 5 Kyle M Hall (khall) 2015-05-08 14:17:42 UTC 
(In reply to Marcel de Rooy from comment #4)
> Hey Julian,
> I am not sure about passing QA on this patch.
> You only actually do some trivial tests for IsOverDriveEnabled.
> The real stuff with requests, UserAgent etc. (probably mocking OverDrive
> behavior somehow) is not there.

While I agree, I say any coverage is better than no coverage! This won't stop additional tests from being added later.
Comment 6 Kyle M Hall (khall) 2015-05-08 14:22:20 UTC 
Created attachment 38991 [details] [review]
[PASSED QA] Bug 14147: Add unit tests to C4::External::OverDrive

This patch test 'IsOverDriveEnabled' subroutine

    TEST PLAN
    ---------
    1. Apply patch
    2. prove -v t/External/OverDrive.t
    -- All 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  :  47,5%
    Branch     :   0,0%
    Condition  :   0,0%
    Subroutine :  75,0%

    Coverage AFTER this patch :
    Statement  :  49,1%
    Branch     :   0,0%
    Condition  :  33,3%
    Subroutine :  83,3%

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tests tun, no koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi (tcohen) 2015-05-14 14:20:44 UTC 
Patch pushed to master.

Thanks Julian!