Bug 14147

Summary: Add unit tests to C4::External::OverDrive
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, kyle, m.de.rooy, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Attachments: Bug 14147 : Add unit tests to C4::External::OverDrive
Bug 14147 : Add unit tests to C4::External::OverDrive
[SIGNED-OFF] Bug 14147: Add unit tests to C4::External::OverDrive
[PASSED QA] Bug 14147: Add unit tests to C4::External::OverDrive

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 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 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 2015-05-14 14:20:44 UTC
Patch pushed to master.

Thanks Julian!