On Sun, Sep 1, 2019 at 12:40 PM Aryeh Gregor <ayg(a)aryeh.name> wrote:
On Fri, Aug 30, 2019 at 10:09 PM Krinkle
<krinklemail(a)gmail.com> wrote:
For anything else, it doesn't really work in
my experience because
PHPUnit
won't actually provide a valid implementation
of the interface. It
returns
null for anything, which is usually not a valid
implementation of the
contract the class under test depends on. [..]
In my experience, classes that use a particular interface usually call
only one or two methods from it, which can usually be replaced by
returning fixed values or at most two or three lines of code. [..]
It could sometimes not be worth it to mock for the reason you give,
but in my experience that's very much the
exception rather than the rule.
[..]
Consider a class like ReadOnlyMode. It has an
ILoadBalancer injected.
The only thing it uses it for is getReadOnlyReason(). When testing
ReadOnlyMode, we want to test "What happens if the load balancer
returns true? What about false?" A mock allows us to inject the
required info with a few lines of code.
While I do prefer explicit dependencies, an extreme of anything is rarely
good. For this case, I'd agree and follow the same approach.
And I think there are plenty of other examples where I'd probably go for a
partial mock. Doing so feels like a compromise to me, as I have often seen
this associated with working around technical debt (e.g. the relationship
between those classes could've been better perhaps). But I think it's fair
to say there will be cases where the relationship is fine and it still
makes sense to write a partial mock to keep the test simple.
In addition to that, I do think it is important to also have at least one
test case at the integration level to verify that the higher-level purpose
and use case of the feature works as intended - where you'd have to
construct the full dependency tree and mock or stub only the signal that is
meant to travel through the layers. Thus ruling out any mistake in the
(separate) unit tests for ReadOnlyMode and LoadBalancer. But, you wouldn't
need to have full coverage through that - just test the communication
between the two. The unit tests would then aim for coverage of all the edge
cases and variations.
Thanks for writing this up.
5) In some cases you want to know that a certain
method is being
called with certain parameters,[ ..] Maybe the bug changed the
WHERE clause to one that happens to select your row of test data
despite being wrong.
Yep, this is important. I tend to go for lower level observation instead of
injected assertions. E.g. mock IDatabase::select/insert etc. to stash each
query in an array, and then toward the end assert that the desired queries
have occurred exactly as intended and nothing more. Similar to how one
can memory-buffer a Logger instance.
I like the higher confidence this gives and I find it easier to write than
a PHPUnit
mock where each function call is precisely counted and params validated,
while being more tolerant to internal details changing over time.
Having said that, they both achieve the same goal. I suppose its a matter
of taste. Certainly nothing wrong with the PHPUnit approach, I'd just not
do it myself as much. Thanks for reminding me of this.
6) [..]
One way to help catch inadvertent performance regressions is to test
that the means of ensuring performance are being used properly. For
instance, if a method is supposed to first check a cache and only fall
back to the database for a cache miss, we want to test that the
database query is actually only issued in the event of a cache miss.
Yep, that works. I tend to do this differently, but it works and its
important
that we make these assertions somewhere. I don't disagree :)
-- Timo