Bug 18899

Summary: Tests should skip instead of failing when dependency modules are not installed
Product: Koha Reporter: Lee Jamison <ldjamison>
Component: Test SuiteAssignee: Lee Jamison <ldjamison>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: mtompset
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18897
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 18899 - Tests should skip instead of failing when dependency modules are not installed

Description Lee Jamison 2017-07-05 20:49:54 UTC
t/db_dependent/00-strict.t (tests 342, 521, 533) and t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t (test 1) fail instead of skipping when a Perl module required for them to run properly is not installed.

test 342 requires Catmandu::Importer::MARC
tests 521 and 533 require WebService::ILS::OverDrive::Patron
test 1 requires Catmandu::Store::ElasticSearch
Comment 1 Lee Jamison 2017-07-06 16:35:06 UTC
Created attachment 64858 [details] [review]
Bug 18899 - Tests should skip instead of failing when dependency modules are not installed

Test Plan
1. Run 'prove -v t/db_dependent/00-strict.t' on
   a fresh Koha box.
2. Notice the failures listed caused by the missing modules
   in the Bug Description and number of tests ran.
3. Apply patch.
4. Re-run 'prove -v t/db_dependent/00-strict.t'.
5. Notice all tests pass and there are 1-3 less tests ran
   (skipped)
Comment 2 Mark Tompsett 2017-07-07 00:25:53 UTC
Comment on attachment 64858 [details] [review]
Bug 18899 - Tests should skip instead of failing when dependency modules are not installed

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

You write:
test 342 requires Catmandu::Importer::MARC
tests 521 and 533 require WebService::ILS::OverDrive::Patron
test 1 requires Catmandu::Store::ElasticSearch

But then your code has no explicit mention of them. I think this needs some logic clean up, sorry.
Even comments explaining the relationship between the requires done and why they are failing (the ones above) would be an improvement.

::: t/db_dependent/00-strict.t
@@ +31,5 @@
> +if ( $@ ) {
> +    push @skips, 'opac/svc/overdrive';
> +}
> +
> +eval { require Koha::ExternalContent::OverDrive; };

This is identical to line 29.
Why didn't you place line 38 as line 33? That is do both pushes in the single $@.
Comment 3 Mark Tompsett 2017-07-07 01:27:02 UTC
Interesting... I came across the same problem a while back, and hadn't gotten to documenting fixing it

File: test_on_fresh_install_failures.txt
"Catmandu/Importer/MARC.pm
 WebService/ILS/OverDrive/Patron.pm 
 Catmandu/Store/ElasticSearch.pm"
Comment 4 Lee Jamison 2018-05-29 15:16:41 UTC
As of the time of this comment the tests no longer fail but instead skip if modules are not installed.