Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/question: Verifying that calls are made in order. #130

Closed
ezzatron opened this issue Sep 18, 2014 · 8 comments
Closed

Feature/question: Verifying that calls are made in order. #130

ezzatron opened this issue Sep 18, 2014 · 8 comments

Comments

@ezzatron
Copy link

I'm trying to migrate from Phake, and one feature I can't find a replacement for is Phake::inOrder().

The basic idea being that unless method calls happen in a specific order, the assertion fails. I had a look into writing my own prediction, but they seem to be tied to a specific method, and this type of assertion potentially spans multiple methods, or even objects.

I'm not really a huge fan of the way Phake handles this anyway. Phake::inOrder() will ignore other method calls in between the ones you specify, which is not always desirable. Sometimes you want to verify that an exact set of calls were made, and no others.

A really good example of some call order verification tools is Sinon.JS's spy API and spy call API. Amongst other features they support things like:

  • spy.firstCall
  • spy.secondCall
  • spy.thirdCall
  • spy.lastCall
  • spy.calledBefore(anotherSpy)
  • spy.calledAfter(anotherSpy)
  • spyCall = spy.getCall(n)

Is there any existing feature that helps with this kind of verification? Or would such features be worth making a PR?

@sstok
Copy link
Contributor

sstok commented Sep 18, 2014

I believe this feature is not supported at the moment, I know PHPUnit Mocks support this.

But personally I'm not a big fan of this kind spying as it makes tests more fragile.
Every call (no mater the method name) is used as a position, which means that when in the future you change the order or (when refactoring) then you're tests will fail even though the code would still work.

You're tests should verify the outcome, in you need test if something is done particular order you can not refactor easily and it makes the tests know to much about the internals.

@stof
Copy link
Member

stof commented Sep 18, 2014

I'm not a big fan of this either.

Btw, it looks like it goes against the Prophecy philosophy. I suggets you to read http://everzet.com/post/72910908762/conceptual-difference-between-mockery-and-prophecy about it

@davedevelopment
Copy link

Ordered expectations usually lead to overspecification IMO, but I can understand why you might want to use them. The naive example I use is:

$em->persist(new User);
$em->flush();

I feel comfortable leaving the order precision out, but then I usually have a higher level test covering it.

@ezzatron
Copy link
Author

I can appreciate that it makes tests more fragile, but I'd argue that there are situations in which it can be very important.

Take for example a mocked file system, passed as a dependency to your class. You want to test that your class checks for the existence of a parent directory before creating a file. If the parent directory does not exist it should create it, but it's important that it do so before creating the file. So you write a test:

    public function testWriteConfig()
    {
        $fs = Phake::mock('FileSystem');
        Phake::when($fs)->directoryExists('/path')->thenReturn(false);

        $configWriter = new ConfigWriter($fs);
        $configWriter->writeConfig('/path/config', new Config);

        Phake::inOrder(
            Phake::verify($fs)->createDirectory('/path'),
            Phake::verify($fs)->writeFile('/path/config', '{}')
        );
    }

If you can't test the order of these operations, you could easily have a runtime error that doesn't show up in your tests. The same could be applied to a mocked database, where referential integrity constraints force you to insert rows in a certain order. There are plenty of similar use cases.

So, what is the solution to thoroughly test a situation like this? Or do you just not test it... thoroughly (which feels gross)?

@docteurklein
Copy link
Contributor

My 2 cents, but I also feel it goes against OOP practices.

Testing order of calls is totally procedural, and an implementation detail.
Only the outome of the public api should be tested.
if your collaborator methods needs either to be called in a specific order or fails, it might be a code smell, telling your collaborator should have a less procedural, side-effect-free api.

Moreover, those kind of problems could be detected by integration tests.
Again my 2 cents, but I never feeled the need to test the order of calls until now.

@sukei
Copy link

sukei commented Oct 11, 2014

I'm currently working on an event dispatcher which calls listeners based on their priority level. The order of made calls is part of the specifications of the dispatcher. For such a case, testing order seems legit.

@everzet
Copy link
Member

everzet commented Nov 29, 2014

As @stof said - order goes against the Prophecy philosophy like it or not. Prophecy intentionally doesn't care in which order your methods are called, all it cares about is message passing behaviour.

That said, describing order using purely message passing is possible. @davedevelopment's example with Prophecy will look something like this:

$em->persist($user)->will(function() {
    $this->flush()->shouldBeCalled();
}))->shouldBeCalled();

It looks ugly and that's intentional. As @docteurklein and others said - procedural style is something Prophecy tries hard to discourage. In rare cases where you do care about order of calls and its not an OOP smell it's usually better to not mock things and do a proper integration test.

You can read more on message passing philosophy of Prophecy here:
http://everzet.com/post/72910908762/conceptual-difference-between-mockery-and-prophecy

@Bilge
Copy link

Bilge commented Dec 21, 2018

@everzet's hack doesn't seem to even work any more. There appears to be no way to test call order reliably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants