Partial Mocking

[hlegius]

Hi guys, There is any news about Partial Mocking supported in v1 ? Here is an example:

function it_will_call_b_and_delegate_to_a() {
  $this->a('foo', 'bar')->shouldBeCalled();
  $this->b('bar');
}

public function a($foo, $bar) {
  // do a
}

public function b($bar) {
  // do something to create foo
  $this->a($foo, $bar);
}

With partial mocks will be possible to mocking $this public methods.

[jakzal]

Prophecy is not going to support partial mocks as mocking a class you're testing is a bad practice and indicates your class has too many responsibilities. For a full discussion see #61.

[hlegius]

@jakzal so:

public function readFromFile($fp);
public function readFromBlackMagic($magicInput);
public function readFromString($string);

So in this case, I can't simple create decorators methods who does something different and delegate the basic behavior (read from string) to a public method that will do it?

The very first rule of Law of Demeter is about it: You can play with yourself.

[jakzal]

@hlegius you delegate to the class collaborators, not the class itself. Please read comments under #61 as everything has been already explained in there.

I don't see how mocking the class under test is related to the law of demeter.

[hlegius]

@jakzal I read it. I agree with @MarcelloDuarte about design complexity and the way how PHPSpec goes - but, it's not about mocking entire self. It's about to able mocking (not stub) a method of self.

As in on PHPSpec, I should create another 2 classes: FileReader and BlackMagicReader and decorate them with StringReader. In some cases, make sense but in others, not. I've pointed Law of Demeter because I can't find any references to refer that decorate methods could be a design smell.

[onlab]

Why do not allow it? The application design is not a problem of the testing framework. You can ensure a minimum principles of OOP but not to require a over bloating of design patterns in the tested application.

[pjedrzejewski]

@onlab @hlegius IMO phpspec is all about application design. Most likely your class should use a collaborator to access the filesystem. From you method names, I'm guessing that Adapter pattern would be a better solution than accessing different data sources from one object.

This is why phpspec is awesome, it does not allow you to do this, because it is wrong design.

[onlab]

@pjedrzejewski I understand your point. But I keep with my opinion that the testing framework should not make you to use patterns when you don't want to use. The framework itself can use the patterns you want, but not the application you're testing. This really doesn't make sense.

[hlegius]

@jakzal @pjedrzejewski fair. It's really awesome. I really do! - but I have concerns with this phpspec's "limitation" force us to do premature over design in some cases. I believe that phpspec would guide us to build a better ood and partial mocks could be one of them.

As an example: RSpec mock engine and Mocha both support mocking any class/object even if not passed as method's dependency. e.g.

subject { Foo.new }

it "will do a mess" do
  expect(SmellClass.new).to receive(:do_something).and_return "bar"
  subject.do_presomething
end

class Foo

  def do_presomething
    SmellClass.new.do_something
  end
end

This is SO wrong and its well documented in Better Specs, et al, but both mock engines support them. My point is: IMO, in some cases, is better have the resource than not.

[jakzal]

Prophecy is a framework developed for phpspec. Phpspec is a tool for test driven development, which in turn is a design activity. If you look at it from this perspective, a design is a concern of a testing framework.

I love the fact that whenever I think of doing something wrong, phpspec and prophecy tell me to get back on the right path. If you don't want to design properly, phpspec is not the tool for you. There's no such thing as "over design".

By the way, you can still use other mocking frameworks with phpspec if you're into that kind of things.

[hlegius]

There is no such thing as "over design"? Inflate domain with tons of patterns to solve a simple thing that ood solve isn't over design? IMO, was better you told me that "phpspec won't support it and done." than this.

Well, thanks guys for all conversation around it.

[onlab]

@jakzal If you want to over design (yeah.. let me say... this exists) PHP is not a language to develop with. What you're asking for is "ignore all PHP tools and do your own adapter thinking in Java programming".

Why.. WHY can't I just use fopen using my own stream without being forced to create a decorator to test it when using it in some component of MY OWN project?

If you tell me that this can't be done because some language deficiency is ok. But your answer is just "I don't want to let anyone use this feature just because I DON'T WANT". It's ridiculous.

[jakzal]

@hlegius that's a bad design you're talking about.

[egoista]

Pray for the holy grail of programming, one solution to rule over all.

[hlegius]

@jakzal we can discuss this all day and both of us can provide a tons of links that assert our point of view. Design is about thinking and make decisions. PHPSpec and Prophecy as a tool should support. Another example is PHPSpec isn't support assert scalar values. Test behavior has its cases, as Sandi Matz described in Lunch'n Learn with Sandi Metz. There she talks about when use mock to spec and state.

Your tool your rules. I already got it.

[pjedrzejewski]

I think it all boils down to the fact that both Prophecy and PhpSpec are highly opinionated tools and if you have different approach to design, it may be not the right tool for you. That's all.

This is a very basic case, where your requirement to mock part of the class indicates too wide responsibility problem, which is the simplest example of wrong design. One of main phpspec's advantages is not allowing you to go this way or making your life harder when you try to write smelly code.

[onlab]

You still didn't answered my question.

Why.. WHY can't I just use fopen using my own stream without being forced to create a decorator to test it when using it in some component of MY OWN project?

If you tell me that this can't be done because some language deficiency is ok. But your answer is just "I don't want to let anyone use this feature just because I DON'T WANT". It's ridiculous.

[danilofreitas]

I also need this feature.

[mathop]

@danilofreitas +1 :+1:

[hlegius]

@pjedrzejewski what you mean with highly opinionated tools ? TDD enforces better design, not a tool. Anyway, PHPSpec could help on this task but assume that testing scalar values or define an expectation on self in some cases is so wrong - it's seems a little bit overreacting attitude, don't you?

Great OO designers already told about BDD Spec testing isn't about testing only behavior - or should I have PHPSpec integrated with PHPUnit or change PHPSpec's mock engine ? I don't think so.

[stof]

Why.. WHY can't I just use fopen using my own stream without being forced to create a decorator to test it when using it in some component of MY OWN project?

if your class is accessing the filesystem, your collaborator is the filesystem, not the PHP function file_exists (we cannot mock functions). And in such case, the right way to go is to use vfsStream to mock the filesystem (or to use the real filesystem if you prefer) in an integration test. So the case of having a protected method fileExists wrapping file_exists just for mocking purpose is a wrong use case for partial mocking IMO.

And the example given at the beginning of the discussion is not a good use case either, as it is the perfect use case for extracting a collaborator instead of putting a separate public method in the same class.

[edsonmedina]

I know I'm late to this (closed) debate, and I'm not going to try to defend any of the sides. Just a quick question.

Am I right to assume that not being able to partial mock (by design) means that you recommend we never use $this->someMethod()?

[stof]

@edsonmedina you can call other methods of the same interface, but this should be unknown for the outside i.e. you don't consider the other method of the same class as a collaborator which should be mocked. Using a partial mock is almost always a sign of mixing multiple responsibilities in the class (as you want to test only of them them by mocking the other)

[edsonmedina]

Ok, so the recommendation is we should test the class just by using the public API as a seam.

Using a partial mock is almost always a sign of mixing multiple responsibilities in the class (as you want to test only of them them by mocking the other)

I see. I do agree that's usually the case. Protected / private methods are usually an indicator that something should be extracted into a different class.

I do understand where the opposition is coming from though. It is a quite radical stance. Not bad, but radical.

Thanks for the enlightenment.

[stof]

Well, nothing prevents you to use a PHPUnit mock for the case where you need a partial mock, and Prophecy for all other mocks. Prophecy is an opinionated tool (as is PhpSpec). Making it hard or impossible to work with badly designed code is actually a feature in PhpSpec, not an issue (even though some people may not agree, in which case they can totally use a different tool).

[edsonmedina]

Maybe that should be phrased in prophecy's github page.

It took me a while (and quite a bit of confusion / frustration) googling around until I realized I wasn't doing anything wrong, and it was a limitation by design.

If you mention you don't do partial mocks, you might save some people the same frustration.