[Koha-bugs] [Bug 10508] UT: C4::Branch.pm needs unit tests

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10508

--- Comment #25 from Galen Charlton <gmcharlt at gmail.com> ---
(In reply to Srdjan Jankovic from comment #24)
> (In reply to Galen Charlton from comment #23)
> > Similar to the concerns I expressed in bug 10528, I have reservations about
> > a patch that both changes a bunch of core routines and adds tests.  Please
> > split it up.
> 
> Why? I think it is a good thing to add tests that support the change. Makes
> it kind of complete

My statement, taken out of the context of this bug, was indeed ill-worded.  Of
course I want tests to accompany patches that change core routines (although I
would prefer that the tests, including regression tests, be in a separate patch
from the one that changes the routines).

But consider it in the context of this bug, which advertises itself as "UT:
C4::Branch.pm needs unit tests", not "there are inappropriate uses of
$sth->finish() in C4::Branches".  In other words, this is part of the ongoing
process to improve test coverage, and it's mixing up *that* with changes to
core routines that I object to.

Sure, in the process of writing tests for a module, it's quite likely that one
will run into cases where a routine ought to be improved -- but those should be
handled separately, either via separate bugs or at least separate patches.

-- 
You are receiving this mail because:
You are watching all bug changes.