Summary: | Increasing test coverage for C4::Log | ||
---|---|---|---|
Product: | Koha | Reporter: | Merllissia Manueli <manuelimel> |
Component: | Test Suite | Assignee: | Chris Cormack <chris> |
Status: | CLOSED FIXED | QA Contact: | Galen Charlton <gmcharlt> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | dhorsman, liz |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
proposed patch
bug 11537: Increasing test coverage for C4::Log bug 11537: Increasing test coverage for C4::Log bug 11537: Increasing test coverage for C4::Log [PASSED QA] Bug 11537: Increasing test coverage for C4::Log |
Description
Merllissia Manueli
2014-01-13 22:51:37 UTC
Created attachment 24243 [details] [review] proposed patch Created attachment 24244 [details] [review] bug 11537: Increasing test coverage for C4::Log To test: 1. run prove t/Log.t Notice 1 test is run 2. apply patch 3. run prove t/Log.t Notice 3 tests are run Signed-off-by: Liz Rea <liz@catalyst.net.nz> 3 tests are in fact run. Thanks for the patch! Comment on attachment 24244 [details] [review] bug 11537: Increasing test coverage for C4::Log Removing, as signoff has to be outside catalyst. Created attachment 24245 [details] [review] bug 11537: Increasing test coverage for C4::Log To test: 1. run prove t/Log.t Notice 1 test is run 2. apply patch 3. run prove t/Log.t Notice 3 tests are run Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. I think there might be a typo in "Testing git log status". Should this read "Testing get log status"? (Noticed while looking at the code/running perl t/Log.t.) In any case, thanks for the patch. Looks good! Created attachment 24246 [details] [review] bug 11537: Increasing test coverage for C4::Log To test: 1. run prove t/Log.t Notice 1 test is run 2. apply patch 3. run prove t/Log.t Notice 3 tests are run Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. I think there might be a typo in "Testing git log status". Should this read "Testing get log status"? (Noticed while looking at the code/running perl t/Log.t.) In any case, thanks for the patch. Looks good! Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Fixed typo, thanks David Created attachment 24268 [details] [review] [PASSED QA] Bug 11537: Increasing test coverage for C4::Log To test: 1. run prove t/Log.t Notice 1 test is run 2. apply patch 3. run prove t/Log.t Notice 3 tests are run Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. I think there might be a typo in "Testing git log status". Should this read "Testing get log status"? (Noticed while looking at the code/running perl t/Log.t.) In any case, thanks for the patch. Looks good! Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Fixed typo, thanks David Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described: 1..3 ok 1 - use C4::Log; ok 2 - Testing GetLogStatus ok 3 - Testing hash is non empty Welcome Alex! Pushed to master. Congratulations on having your first patch accepted into Koha, Alex! Upon Jenkins (the continuous integration service) complaining, I've also pushed a follow-up that mocks the calls to C4::Context::preference() that C4::Log::GetLogStatus() makes, which will allow the test to pass without requiring that a Koha database is present and pointed to by a valid Koha configuration. |