Bug 9053 - Test::MockModule and DBD::Mock should be required
Summary: Test::MockModule and DBD::Mock should be required
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Jared Camins-Esakov
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-10 17:21 UTC by Jared Camins-Esakov
Modified: 2019-06-27 09:24 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:


Attachments
Bug 9053: Test::MockModule and DBD::Mock should be required (1.02 KB, patch)
2012-11-10 19:04 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9053: Test::MockModule and DBD::Mock should be required (1.07 KB, patch)
2012-11-11 06:14 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-11-10 17:21:18 UTC

    
Comment 1 Jared Camins-Esakov 2012-11-10 19:04:36 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2012-11-11 06:14:02 UTC
Created attachment 13379 [details] [review]
Bug 9053: Test::MockModule and DBD::Mock should be required

Test::MockModule and DBD::Mock are required for the unit tests in t/
to run. This is good, but means they should be required.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 3 Jonathan Druart 2012-11-12 11:17:03 UTC
QA comment:

The patch is correct but I don't understand where stands the boundary between "required" and "not required". What is the idea behind this patch?

To me, launching tests is not useful for a fonctionnal use of Koha.
If these 2 modules are required perhaps should we add the others Test::* as required, too?
Comment 4 Jared Camins-Esakov 2012-11-12 13:28:09 UTC
(In reply to comment #3)
> QA comment:
> 
> The patch is correct but I don't understand where stands the boundary
> between "required" and "not required". What is the idea behind this patch?
> 
> To me, launching tests is not useful for a fonctionnal use of Koha.
> If these 2 modules are required perhaps should we add the others Test::* as
> required, too?

These two modules are required for the tests in t/ to pass -- which is reasonable -- and running the tests in t/ is part of the installation process. I don't know that it's necessary to make all Test modules required, though I don't know that there's any reason *not* to make all Test modules required. Installing the Test modules is not a great hardship, and I'd rather new users know if there's a problem with the version of Koha that they're installing.
Comment 5 Jonathan Druart 2012-11-12 14:34:36 UTC
(In reply to comment #4)
> These two modules are required for the tests in t/ to pass -- which is
> reasonable -- and running the tests in t/ is part of the installation
> process. I don't know that it's necessary to make all Test modules required,
> though I don't know that there's any reason *not* to make all Test modules
> required. Installing the Test modules is not a great hardship, and I'd
> rather new users know if there's a problem with the version of Koha that
> they're installing.

Thanks for your answer.

Marked as Passed QA.
Comment 6 Jared Camins-Esakov 2012-11-12 14:49:24 UTC
This patch has been pushed to master.
Comment 7 Paul Poulain 2012-11-16 17:20:40 UTC
Patch pushed to branch 3.10.x
Comment 8 Chris Cormack 2013-04-22 08:47:24 UTC
Released in 3.10.0